From 13b4168a27b6aeb01837ce12bb9da64cb991621d Mon Sep 17 00:00:00 2001 From: Daniel Song Date: Tue, 3 Mar 2026 21:11:20 -0800 Subject: [PATCH 1/3] fix: use target precision in Presto timestamp rounding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rounds=True and rounds=False branches in Presto's normalize_timestamp produced identical SQL, silently ignoring the rounding flag. Cast to coltype.precision (not 6) when rounding, matching Trino's correct implementation. Also clarify the misleading TODO in Join.select — column validation is already handled by resolve_names(). Closes #13 Co-Authored-By: Claude Opus 4.6 --- data_diff/databases/presto.py | 3 +-- data_diff/queries/ast_classes.py | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/data_diff/databases/presto.py b/data_diff/databases/presto.py index 339f49ec..e6a5b294 100644 --- a/data_diff/databases/presto.py +++ b/data_diff/databases/presto.py @@ -126,9 +126,8 @@ def normalize_uuid(self, value: str, coltype: ColType_UUID) -> str: return f"TRIM(CAST({value} AS VARCHAR))" def normalize_timestamp(self, value: str, coltype: TemporalType) -> str: - # TODO rounds if coltype.rounds: - s = f"date_format(cast({value} as timestamp(6)), '%Y-%m-%d %H:%i:%S.%f')" + s = f"date_format(cast({value} as timestamp({coltype.precision})), '%Y-%m-%d %H:%i:%S.%f')" else: s = f"date_format(cast({value} as timestamp(6)), '%Y-%m-%d %H:%i:%S.%f')" diff --git a/data_diff/queries/ast_classes.py b/data_diff/queries/ast_classes.py index 2df227dc..9cf91fcf 100644 --- a/data_diff/queries/ast_classes.py +++ b/data_diff/queries/ast_classes.py @@ -515,7 +515,7 @@ def select(self, *exprs, **named_exprs) -> Self | ITable: named_exprs = _drop_skips_dict(named_exprs) exprs += _named_exprs_as_aliases(named_exprs) resolve_names(self.source_table, exprs) - # TODO Ensure exprs <= self.columns ? + # Column validation is handled by resolve_names() above return attrs.evolve(self, columns=exprs) From 9507ea7635967be14ffaad41de2ec89a35ceb612 Mon Sep 17 00:00:00 2001 From: Daniel Song Date: Tue, 3 Mar 2026 21:14:16 -0800 Subject: [PATCH 2/3] fix: clarify comment wording per review feedback resolve_names() performs name resolution (raises on unknown names), not general column validation. Updated comment to accurately describe the behavior. Co-Authored-By: Claude Opus 4.6 --- data_diff/queries/ast_classes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data_diff/queries/ast_classes.py b/data_diff/queries/ast_classes.py index 9cf91fcf..38863ce2 100644 --- a/data_diff/queries/ast_classes.py +++ b/data_diff/queries/ast_classes.py @@ -515,7 +515,7 @@ def select(self, *exprs, **named_exprs) -> Self | ITable: named_exprs = _drop_skips_dict(named_exprs) exprs += _named_exprs_as_aliases(named_exprs) resolve_names(self.source_table, exprs) - # Column validation is handled by resolve_names() above + # No explicit subset check needed; resolve_names() will raise if any column name is invalid return attrs.evolve(self, columns=exprs) From e2e727c514b9dde2ba57ddfc3bbabf3d36b79ace Mon Sep 17 00:00:00 2001 From: Daniel Song Date: Tue, 3 Mar 2026 21:33:30 -0800 Subject: [PATCH 3/3] fix: specify exception type and condition in comment resolve_names() raises KeyError (not a generic error) and only validates when schema is present. Co-Authored-By: Claude Opus 4.6 --- data_diff/queries/ast_classes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/data_diff/queries/ast_classes.py b/data_diff/queries/ast_classes.py index 38863ce2..8e6053a6 100644 --- a/data_diff/queries/ast_classes.py +++ b/data_diff/queries/ast_classes.py @@ -515,7 +515,7 @@ def select(self, *exprs, **named_exprs) -> Self | ITable: named_exprs = _drop_skips_dict(named_exprs) exprs += _named_exprs_as_aliases(named_exprs) resolve_names(self.source_table, exprs) - # No explicit subset check needed; resolve_names() will raise if any column name is invalid + # No explicit subset check needed; resolve_names() raises KeyError for invalid column names (when schema is present) return attrs.evolve(self, columns=exprs)