-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FEATURE] Support to include ID/PK in validation result for each row - SQL #6448
Merged
Merged
Changes from 52 commits
Commits
Show all changes
56 commits
Select commit
Hold shift + click to select a range
017350c
Before the fix
2af5d8d
Merge branch 'develop' into b/dx-67/bugfix-metrics-return-empty-value
43d30ac
cleaned up db references
0c93b5d
Merge branch 'develop' into b/dx-67/bugfix-metrics-return-empty-value
015b84f
bugfix
65c3e3a
Merge branch 'b/dx-67/bugfix-metrics-return-empty-value' of https://g…
61867c2
updated etst to not include extra comments or output
c2043bd
Update test_map_metric.py
c06b838
Merge branch 'develop' into b/dx-67/bugfix-metrics-return-empty-value
583f89d
Update test_map_metric.py
93ae465
Update test_map_metric.py
3b983a6
update column names
712d9e2
Merge branch 'develop' into b/dx-67/bugfix-metrics-return-empty-value
cb54172
Merge branch 'develop' into b/dx-67/bugfix-metrics-return-empty-value
7dd34c3
oops
75cc9b2
Sql Metrics added from other PR
5a3b22f
Merge branch 'develop' into b/dx-67/sql-pk-id-metric
f5d1a05
Update test_map_metric.py
6ab5d3e
see if this works for now
1cfc4df
push before final check
9bfdde1
fixed
2447162
Merge branch 'develop' into b/dx-67/sql-pk-id-metric
5a11c0e
cleaned up
75134e9
cleaned up final function
7f03f3d
very large
c4c78c7
final clean up
1c4d0b9
make it shorter
4b18bc7
Update map_metric_provider.py
ab9be1f
simplifying the filtering
94e54a9
whoohoo
a2c7fcb
let's add some tests
5d9c923
Merge branch 'develop' into b/dx-67/sql-pk-id-metric
b8c06b1
Update test_metrics_util.py
145c806
Update map_metric_provider.py
1b1163f
Update test_metrics_util.py
1bf4bf8
last fix
f75ea66
AB testing
a8a5997
final set
1090b07
much cleaner
159966f
athena needs special treatment
97cf510
Merge branch 'develop' into b/dx-67/sql-pk-id-metric
2783c04
monkeypatch everything
8ca9b3f
Update test_metrics_util.py
47caaca
wow what was that
9e2c00d
ok so we here go again
ed131f1
i think i finally haev it this time
6deb72e
Update metrics_test.db
72494f9
Update test_metrics_util.py
7d63cd2
now the tests are fixed
071cf19
q1
40cf1f6
clean up of tests
ba177f6
Merge branch 'develop' into b/dx-67/sql-pk-id-metric
c5e9741
Revert "clean up of tests"
2eb7aa9
Revert "Revert "clean up of tests""
2abc598
updated after synchronous review
2fe0627
Merge branch 'develop' into b/dx-67/sql-pk-id-metric
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,8 +29,9 @@ | |
|
||
try: | ||
import sqlalchemy as sa | ||
from sqlalchemy import Table | ||
from sqlalchemy.dialects import registry | ||
from sqlalchemy.engine import Engine, reflection | ||
from sqlalchemy.engine import Connection, Engine, reflection | ||
from sqlalchemy.engine.interfaces import Dialect | ||
from sqlalchemy.exc import OperationalError | ||
from sqlalchemy.sql import Insert, Select, TableClause | ||
|
@@ -47,6 +48,7 @@ | |
sa = None | ||
registry = None | ||
Engine = None | ||
Connection = None | ||
reflection = None | ||
Dialect = None | ||
Insert = None | ||
|
@@ -992,3 +994,70 @@ def is_valid_continuous_partition_object(partition_object): | |
and np.all(np.diff(partition_object["bins"]) > 0) | ||
and np.allclose(np.sum(comb_weights), 1.0) | ||
) | ||
|
||
|
||
def sql_statement_with_post_compile_to_string( | ||
engine: SqlAlchemyExecutionEngine, select_statement: "sqlalchemy.sql.Select" | ||
) -> str: | ||
""" | ||
Util method to compile SQL select statement with post-compile parameters into a string. Logic lifted directly | ||
from sqlalchemy documentation. | ||
|
||
https://docs.sqlalchemy.org/en/14/faq/sqlexpressions.html#rendering-postcompile-parameters-as-bound-parameters | ||
|
||
Used by _sqlalchemy_map_condition_index() in map_metric_provider to build query that will allow you to | ||
return unexpected_index_values. | ||
|
||
Args: | ||
engine (sqlalchemy.engine.Engine): Sqlalchemy engine used to do the compilation. | ||
select_statement (sqlalchemy.sql.Select): Select statement to compile into string. | ||
Returns: | ||
String representation of select_statement | ||
|
||
""" | ||
sqlalchemy_connection: "sa.engine.base.Connection" = engine.engine | ||
compiled = select_statement.compile( | ||
sqlalchemy_connection, | ||
compile_kwargs={"render_postcompile": True}, | ||
dialect=engine.dialect, | ||
) | ||
dialect_name: str = engine.dialect_name | ||
|
||
if dialect_name in ["sqlite", "trino", "mssql"]: | ||
params = (repr(compiled.params[name]) for name in compiled.positiontup) | ||
query_as_string = re.sub(r"\?", lambda m: next(params), str(compiled)) | ||
|
||
else: | ||
params = (repr(compiled.params[name]) for name in list(compiled.params.keys())) | ||
query_as_string = re.sub(r"%\(.*?\)s", lambda m: next(params), str(compiled)) | ||
|
||
# bigquery inserts extra '`' character for compiled statement. | ||
# clean up string before returning | ||
if dialect_name == "bigquery": | ||
query_as_string = re.sub(r"`", "", query_as_string) | ||
|
||
return query_as_string | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Return SQL query according to backend |
||
|
||
|
||
def get_sqlalchemy_source_table_and_schema( | ||
engine: SqlAlchemyExecutionEngine, | ||
) -> "Table": | ||
""" | ||
Util method to return table name that is associated with current batch. | ||
|
||
This is used by `_sqlalchemy_map_condition_query()` which returns a query that allows users to return | ||
unexpected_index_values. | ||
|
||
Args: | ||
engine (SqlAlchemyExecutionEngine): Engine that is currently being used to calculate the Metrics | ||
Returns: | ||
SqlAlchemy Table that is the source table and schema. | ||
""" | ||
schema_name = engine.batch_manager.active_batch_data.source_schema_name | ||
table_name = engine.batch_manager.active_batch_data.source_table_name | ||
selectable = sa.Table( | ||
table_name, | ||
sa.MetaData(), | ||
schema=schema_name, | ||
) | ||
return selectable |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: adjust test to compare the query from bigquery, rather than adjust the query from bigquery to match the test