Skip to content

Commit 7096624

Browse files
author
Saurabh Badenkal
committed
Upgrade: cross join guardrail from warning to ValidationError
Implicit cross joins (FROM a, b without ON) now raise ValidationError instead of UserWarning. The server allows these but they produce cartesian products (N*M intermediate rows) that degrade shared DB. SDK now blocks with clear error: 'Implicit cross join detected... Use explicit JOIN...ON syntax instead.' New subcode: validation_sql_cross_join_blocked SDK guardrail summary: - INSERT/UPDATE/DELETE -> ValidationError (blocked) - FROM a, b (cartesian) -> ValidationError (blocked) <-- upgraded - LIKE '%value' -> UserWarning (performance advisory) - SELECT * with JOIN -> UserWarning (partial expansion) 756 unit tests passing.
1 parent f8f5151 commit 7096624

3 files changed

Lines changed: 24 additions & 30 deletions

File tree

src/PowerPlatform/Dataverse/core/_error_codes.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
VALIDATION_SQL_NOT_STRING = "validation_sql_not_string"
4343
VALIDATION_SQL_EMPTY = "validation_sql_empty"
4444
VALIDATION_SQL_WRITE_BLOCKED = "validation_sql_write_blocked"
45+
VALIDATION_SQL_CROSS_JOIN_BLOCKED = "validation_sql_cross_join_blocked"
4546
VALIDATION_ENUM_NO_MEMBERS = "validation_enum_no_members"
4647
VALIDATION_ENUM_NON_INT_VALUE = "validation_enum_non_int_value"
4748
VALIDATION_UNSUPPORTED_COLUMN_TYPE = "validation_unsupported_column_type"

src/PowerPlatform/Dataverse/data/_odata.py

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
VALIDATION_SQL_NOT_STRING,
3030
VALIDATION_SQL_EMPTY,
3131
VALIDATION_SQL_WRITE_BLOCKED,
32+
VALIDATION_SQL_CROSS_JOIN_BLOCKED,
3233
METADATA_ENTITYSET_NOT_FOUND,
3334
METADATA_ENTITYSET_NAME_MISSING,
3435
METADATA_TABLE_NOT_FOUND,
@@ -926,15 +927,15 @@ def _sql_guardrails(self, sql: str) -> str:
926927
stacklevel=4,
927928
)
928929

929-
# 3. Warn on implicit cross joins
930+
# 3. Block implicit cross joins
930931
if self._SQL_IMPLICIT_CROSS_JOIN_RE.search(sql):
931-
warnings.warn(
932-
"Query appears to use an implicit cross join "
933-
"(FROM table1, table2). This produces a cartesian product "
934-
"and may return excessive rows. Use explicit JOIN...ON "
935-
"syntax instead.",
936-
UserWarning,
937-
stacklevel=4,
932+
raise ValidationError(
933+
"Implicit cross join detected (FROM table1, table2). "
934+
"This produces a cartesian product that can generate "
935+
"millions of intermediate rows and degrade shared database "
936+
"performance. Use explicit JOIN...ON syntax instead: "
937+
"FROM table1 a JOIN table2 b ON a.column = b.column",
938+
subcode=VALIDATION_SQL_CROSS_JOIN_BLOCKED,
938939
)
939940

940941
return sql

tests/unit/data/test_sql_guardrails.py

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

157157

158158
# ===================================================================
159-
# 4. Warn on implicit cross joins
159+
# 4. Block implicit cross joins
160160
# ===================================================================
161161

162162

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

166-
def test_comma_join_warns(self):
166+
def test_comma_join_raises(self):
167167
c = _client()
168-
with warnings.catch_warnings(record=True) as w:
169-
warnings.simplefilter("always")
168+
with pytest.raises(ValidationError, match="cross join"):
170169
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
173170

174-
def test_explicit_join_no_warning(self):
171+
def test_explicit_join_not_blocked(self):
175172
c = _client()
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
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
183178

184-
def test_single_table_no_warning(self):
179+
def test_single_table_not_blocked(self):
185180
c = _client()
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
181+
result = c._sql_guardrails("SELECT TOP 10 name FROM account")
182+
assert "SELECT" in result
191183

192184

193185
# ===================================================================

0 commit comments

Comments
 (0)