Repository navigation
Conversation
|
|
||
| start_idx: int | None = None | ||
| if not isinstance(start, sqlglot_expressions.Null): | ||
| if isinstance(start, sqlglot_expressions.Literal): |
There was a problem hiding this comment.
SqlGlot changed how it handle negative numbers where it's now represented as unary-minus-of-a-positive-literal instead of a signed literal. So now isinstance(x, Literal) will fail for negative values so created extract_int_literal utility and reused in convert_slice places where it has isinstance... line.
|
|
||
| date1_truc: SQLGlotExpression = sqlglot_expressions.TimestampTrunc( | ||
| this=date1, unit=unit | ||
| this=date1, unit=sqlglot_expressions.Var(this=unit.value) |
There was a problem hiding this comment.
Fixed error ValueError: Unsupported expression type DateTimeUnit
Other dialects already wrapped it, new SqlGlot version is more strict and had a fallback. Now it raises ValueError
| expression = fix_expression_type(expression, dialect) # PyDough change | ||
| expression = add_text_to_concat(expression) | ||
| expression = replace_date_funcs(expression) | ||
| expression = replace_date_funcs(expression, dialect=dialect) |
There was a problem hiding this comment.
Passing dialect is now required
|
|
||
|
|
||
| def quote_oracle_identifiers(expr: SQLGlotExpression, dialect: SQLGlotDialect) -> None: | ||
| def _oracle_identifier_needs_quoting(name: str) -> bool: |
There was a problem hiding this comment.
Refactored code to be able to use twice.
| predicates. | ||
| """ | ||
| for agg_expr in find_all_in_scope(expression, exp.AggFunc, bfs=True): | ||
| for agg_expr in find_all_in_scope(expression, exp.AggFunc): |
There was a problem hiding this comment.
New version dropped bfs parameter. It only does a single traversal order now (walk_in_scope’s own order).
| @@ -1,20 +1,22 @@ | |||
| """ | |||
| Overridden version of the qualify.py and qualify_tables.py file from sqlglot. | |||
| Overridden version of the qualify.py, qualify_tables.py, and qualify_columns.py | |||
There was a problem hiding this comment.
SqlGlot new version restructured these functions so the changes here reflect that.
So all changes were to match upstream version and re-locate and re-apply the 4 PyDough changes to handle these changes.
| # Protect Oracle-reserved-word identifiers from being upper-cased by | ||
| # qualify's normalize_identifiers step, before quote_oracle_identifiers | ||
| # (below) has a chance to quote them with their original casing intact. | ||
| protect_oracle_reserved_identifiers(glot_expr, dialect) |
There was a problem hiding this comment.
In the new SqlGlot version, qualify()'s internal step order changed: normalize_identifiers now runs before table/column qualification, whereas previously it ran after.
This now caused a problem where PyDough's existing quote_oracle_identifiers() (unmodified, pre-existing code) adds quotes around Oracle-reserved-word identifiers like key, date, comment, but it only runs at the very end of the optimizer pipeline. Under the new step order, normalize_identifiers now upper-cases any not-yet-quoted identifier before quote_oracle_identifiers gets a chance to run, so by the time quoting is added, the original casing (key) has already been destroyed (KEY), and the quoted result ("KEY") no longer matches the real column name (key) created earlier via a different code path that did preserve casing.
| left, right = scope.union_scopes | ||
| scope._external_columns = left.external_columns + right.external_columns | ||
| else: | ||
| local_source_names = {name for name, _ in scope.references} |
There was a problem hiding this comment.
Reflect filtering logic changes from selected_sources-based to references-based
| @@ -1,10 +1,10 @@ | |||
| WITH "_T0" AS ( | |||
| WITH "_t0" AS ( | |||
There was a problem hiding this comment.
Most file changes are cosmetic like:
- Upper-casing for Oracle and Snowflake files
- Changes to synthetic alias naming
_q_0/_T1to_0/_t1 - Pretty-print reformatting, line-wrapping/parenthesization
- Function name changes such as
SUBSTRINGtoSUBSTR
| ON _u_0._u_1 = SUBSTRING(nation.n_name, 1, 1) AND _u_0._u_2 = nation.n_regionkey | ||
| WHERE | ||
| _u_0._u_1 IS NULL | ||
| NOT NOT _u_0._u_1 IS NULL |
There was a problem hiding this comment.
This is new behavior on purpose. Sqlglot 30.18 added dialect.SAFE_TO_ELIMINATE_DOUBLE_NEGATION, which is False for SQLite and MySQL specifically (some edge case upstream found unsafe for those engines); old Sqlglot collapsed NOT NOT x unconditionally for every dialect.
| ROW_NUMBER() OVER (PARTITION BY LENGTH(NULLIF(sbtickerexchange, 'NYSE Arca')) ORDER BY '1') - 1.0 | ||
| ROW_NUMBER() OVER ( | ||
| PARTITION BY LENGTH(CASE WHEN SBTICKEREXCHANGE <> 'NYSE Arca' THEN SBTICKEREXCHANGE ELSE NULL END) | ||
| ORDER BY '1' | ||
| ) - 1.0 |
There was a problem hiding this comment.
NULLIF(x,y) now expands to its definitionally-equivalent CASE WHEN x<>y THEN x ELSE NULL END form here because this subexpression matches the outer GROUP BY key. It protects anything matching the GROUP BY key from simplification
| @@ -1,544 +1,544 @@ | |||
| SELECT | |||
| TRUNC(CAST(SUBSTR(sbcustid, 2) AS DOUBLE PRECISION), '0') AS "_expr0", | |||
| TRUNC(CAST(SUBSTR(SBCUSTID, 2) AS DOUBLE PRECISION), 0) AS "_expr0", | |||
There was a problem hiding this comment.
Changes from "0" to 0
Oracle accepts both via implicit coercion, so this was never a correctness bug, just now matches the code's actual intent.
| TO_CHAR(lineitem.l_receiptdate, 'DD-mm-yyyy') AS date_dmy, | ||
| TO_CHAR(lineitem.l_receiptdate, 'mm/DD') AS date_md, | ||
| TO_CHAR(lineitem.l_receiptdate, 'hh24:miAM') AS am_pm |
There was a problem hiding this comment.
redundant casting better caught in the new version of SqlGLot
2 participants
Linked ticket
Closes #553
Type of change
What changed and why?
How I tested this?
Notes for reviewers