Skip to content

Commit 68d32ea

Browse files
author
Saurabh Badenkal
committed
Revert: cross join guardrail back to UserWarning (not ValidationError)
Principle: SDK should not be more restrictive than the server. The server allows cartesian queries (FROM a, b) -- they work and return results (capped at 5000). The SDK warns users but does not block, because: 1. Small-table cartesian products are safe (3 rows * 3 rows = 9) 2. Server has its own resource governance (5000-row cap, timeouts) 3. Blocking what the server allows creates user frustration Final guardrail design: - ValidationError: INSERT/UPDATE/DELETE (server blocks too, but SDK catches earlier with clearer message) - UserWarning: FROM a, b (cartesian), LIKE '%value', SELECT * + JOIN 756 unit tests passing.
1 parent 7096624 commit 68d32ea

2 files changed

Lines changed: 28 additions & 18 deletions

File tree

src/PowerPlatform/Dataverse/data/_odata.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -927,15 +927,17 @@ def _sql_guardrails(self, sql: str) -> str:
927927
stacklevel=4,
928928
)
929929

930-
# 3. Block implicit cross joins
930+
# 3. Warn on implicit cross joins (server allows these but they
931+
# produce cartesian products that can stress shared DB resources)
931932
if self._SQL_IMPLICIT_CROSS_JOIN_RE.search(sql):
932-
raise ValidationError(
933-
"Implicit cross join detected (FROM table1, table2). "
933+
warnings.warn(
934+
"Query uses an implicit cross join (FROM table1, table2). "
934935
"This produces a cartesian product that can generate "
935936
"millions of intermediate rows and degrade shared database "
936937
"performance. Use explicit JOIN...ON syntax instead: "
937938
"FROM table1 a JOIN table2 b ON a.column = b.column",
938-
subcode=VALIDATION_SQL_CROSS_JOIN_BLOCKED,
939+
UserWarning,
940+
stacklevel=4,
939941
)
940942

941943
return sql

tests/unit/data/test_sql_guardrails.py

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -156,30 +156,38 @@ def test_no_like_no_warning(self):
156156

157157

158158
# ===================================================================
159-
# 4. Block implicit cross joins
159+
# 4. Warn on implicit cross joins (server allows, SDK warns)
160160
# ===================================================================
161161

162162

163-
class TestImplicitCrossJoinBlocked:
164-
"""FROM a, b (comma syntax) must be blocked with ValidationError."""
163+
class TestImplicitCrossJoinWarning:
164+
"""FROM a, b (comma syntax) should emit UserWarning (not error)."""
165165

166-
def test_comma_join_raises(self):
166+
def test_comma_join_warns(self):
167167
c = _client()
168-
with pytest.raises(ValidationError, match="cross join"):
168+
with warnings.catch_warnings(record=True) as w:
169+
warnings.simplefilter("always")
169170
c._sql_guardrails("SELECT TOP 10 a.name, c.fullname FROM account a, contact c")
171+
cross_warnings = [x for x in w if "cross join" in str(x.message).lower()]
172+
assert len(cross_warnings) == 1
170173

171-
def test_explicit_join_not_blocked(self):
174+
def test_explicit_join_no_warning(self):
172175
c = _client()
173-
# Should NOT raise
174-
result = c._sql_guardrails(
175-
"SELECT TOP 10 a.name FROM account a " "JOIN contact c ON a.accountid = c.parentcustomerid"
176-
)
177-
assert "JOIN" in result
176+
with warnings.catch_warnings(record=True) as w:
177+
warnings.simplefilter("always")
178+
c._sql_guardrails(
179+
"SELECT TOP 10 a.name FROM account a " "JOIN contact c ON a.accountid = c.parentcustomerid"
180+
)
181+
cross_warnings = [x for x in w if "cross join" in str(x.message).lower()]
182+
assert len(cross_warnings) == 0
178183

179-
def test_single_table_not_blocked(self):
184+
def test_single_table_no_warning(self):
180185
c = _client()
181-
result = c._sql_guardrails("SELECT TOP 10 name FROM account")
182-
assert "SELECT" in result
186+
with warnings.catch_warnings(record=True) as w:
187+
warnings.simplefilter("always")
188+
c._sql_guardrails("SELECT TOP 10 name FROM account")
189+
cross_warnings = [x for x in w if "cross join" in str(x.message).lower()]
190+
assert len(cross_warnings) == 0
183191

184192

185193
# ===================================================================

0 commit comments

Comments
 (0)