Skip to content

Commit a595a72

Browse files
author
Saurabh Badenkal
committed
Address all 11 PR review comments
1. Remove unused imports (patch, PropertyMock) from test_select_star_expansion 2. Fix JOIN depth '6+' -> 'no depth limit' in query.py docstring 3. Fix stale 'auto-inject TOP' comment in _odata.py 4. Fix Learn-incompatible docstring types in tables.py list_columns 5. Fix Learn-incompatible docstring types in tables.py list_relationships 6. Fix list_table_relationships docstring: add ManyToOne 7. Fix cross-join regex to catch unaliased FROM account, contact 8. Fix write regex to catch comment-prefixed writes (/* */ and --) 9. Narrow odata_expands exception handling (KeyError/AttributeError/ValueError) 10. Consistent JOIN depth claims across all docs 770 unit tests passing.
1 parent b18a219 commit a595a72

4 files changed

Lines changed: 15 additions & 15 deletions

File tree

src/PowerPlatform/Dataverse/data/_odata.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -837,12 +837,12 @@ def _do_request(url: str, *, params: Optional[Dict[str, Any]] = None) -> Dict[st
837837

838838
# ----------------------- SQL guardrail patterns --------------------
839839
_SQL_WRITE_RE = re.compile(
840-
r"^\s*(?:INSERT|UPDATE|DELETE|DROP|TRUNCATE|ALTER|CREATE|EXEC|GRANT|REVOKE|BULK)\b",
841-
re.IGNORECASE,
840+
r"^\s*(?:/\*.*?\*/\s*|--[^\n]*\n\s*)*(?:INSERT|UPDATE|DELETE|DROP|TRUNCATE|ALTER|CREATE|EXEC|GRANT|REVOKE|BULK)\b",
841+
re.IGNORECASE | re.DOTALL,
842842
)
843843
_SQL_LEADING_WILDCARD_RE = re.compile(r"\bLIKE\s+'%[^']", re.IGNORECASE)
844844
_SQL_IMPLICIT_CROSS_JOIN_RE = re.compile(
845-
r"\bFROM\s+[A-Za-z0-9_]+\s+[A-Za-z0-9_]+\s*,\s*[A-Za-z0-9_]+",
845+
r"\bFROM\s+[A-Za-z0-9_]+(?:\s+[A-Za-z0-9_]+)?\s*,\s*[A-Za-z0-9_]+",
846846
re.IGNORECASE,
847847
)
848848
_SQL_HAS_JOIN_RE = re.compile(r"\bJOIN\b", re.IGNORECASE)
@@ -1048,7 +1048,7 @@ def _query_sql(self, sql: str) -> list[dict[str, Any]]:
10481048
# Auto-expand SELECT * into explicit column names
10491049
sql = self._expand_select_star(sql, logical)
10501050

1051-
# Apply safety guardrails (block writes, auto-inject TOP, warn on risky patterns)
1051+
# Apply safety guardrails (block unsupported syntax, warn on risky patterns)
10521052
sql = self._sql_guardrails(sql)
10531053

10541054
entity_set = self._entity_set_from_schema_name(logical)

src/PowerPlatform/Dataverse/operations/query.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ def sql(self, sql: str) -> List[Record]:
9999
100100
SELECT / SELECT DISTINCT / SELECT TOP N (0-5000)
101101
FROM table [alias]
102-
INNER JOIN / LEFT JOIN (multi-table, up to 6+ tables)
102+
INNER JOIN / LEFT JOIN (multi-table, no depth limit)
103103
WHERE (=, !=, >, <, >=, <=, LIKE, IN, NOT IN, IS NULL,
104104
IS NOT NULL, BETWEEN, AND, OR, nested parentheses)
105105
GROUP BY column
@@ -481,8 +481,8 @@ def odata_expands(
481481
try:
482482
with self._client._scoped_odata() as od:
483483
target_set = od._entity_set_from_schema_name(target)
484-
except Exception:
485-
pass
484+
except (KeyError, AttributeError, ValueError):
485+
pass # Entity set resolution failed; target_set stays empty
486486

487487
result.append(
488488
{

src/PowerPlatform/Dataverse/operations/tables.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -723,13 +723,13 @@ def list_columns(
723723
:type table: :class:`str`
724724
:param select: Optional list of property names to project via
725725
``$select``. Values are passed as-is (PascalCase).
726-
:type select: :class:`list` of :class:`str` or None
726+
:type select: list[str] or None
727727
:param filter: Optional OData ``$filter`` expression. For example,
728728
``"AttributeType eq 'String'"`` returns only string columns.
729729
:type filter: :class:`str` or None
730730
731731
:return: List of raw attribute metadata dictionaries.
732-
:rtype: :class:`list` of :class:`dict`
732+
:rtype: list[dict[str, typing.Any]]
733733
734734
:raises ~PowerPlatform.Dataverse.core.errors.MetadataError:
735735
If the table is not found.
@@ -774,10 +774,10 @@ def list_relationships(
774774
:type filter: :class:`str` or None
775775
:param select: Optional list of property names to project via
776776
``$select``. Values are passed as-is (PascalCase).
777-
:type select: :class:`list` of :class:`str` or None
777+
:type select: list[str] or None
778778
779779
:return: List of raw relationship metadata dictionaries.
780-
:rtype: :class:`list` of :class:`dict`
780+
:rtype: list[dict[str, typing.Any]]
781781
782782
:raises ~PowerPlatform.Dataverse.core.errors.HttpError:
783783
If the Web API request fails.
@@ -827,9 +827,9 @@ def list_table_relationships(
827827
``$select``. Values are passed as-is (PascalCase).
828828
:type select: :class:`list` of :class:`str` or None
829829
830-
:return: Combined list of one-to-many and many-to-many relationship
831-
metadata dictionaries.
832-
:rtype: :class:`list` of :class:`dict`
830+
:return: Combined list of one-to-many, many-to-one, and many-to-many
831+
relationship metadata dictionaries.
832+
:rtype: list[dict[str, typing.Any]]
833833
834834
:raises ~PowerPlatform.Dataverse.core.errors.MetadataError:
835835
If the table is not found.

tests/unit/data/test_select_star_expansion.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
"""Unit tests for SELECT * auto-expansion in _query_sql."""
55

66
import pytest
7-
from unittest.mock import MagicMock, patch, PropertyMock
7+
from unittest.mock import MagicMock
88

99
from PowerPlatform.Dataverse.data._odata import _ODataClient
1010

0 commit comments

Comments
 (0)