Skip to content

[CONTRIB] Fix row_condition handling for SQLAlchemyExecutionEngine#11742

Open
Mallik544 wants to merge 3 commits intogreat-expectations:developfrom
Mallik544:fix/11319-row-condition-postgres
Open

[CONTRIB] Fix row_condition handling for SQLAlchemyExecutionEngine#11742
Mallik544 wants to merge 3 commits intogreat-expectations:developfrom
Mallik544:fix/11319-row-condition-postgres

Conversation

@Mallik544
Copy link
Copy Markdown

Summary

Fixes row_condition handling in SqlAlchemyExecutionEngine by wrapping non-table selectables in a subquery before applying the filter.

Problem

When applying row_condition, the code used select_from(selectable) directly. For non-table selectables, this could generate malformed SQL or fail under SQLAlchemy 2.0-style handling.

Changes

  • added a guard to convert non-table/non-subquery selectables into a subquery
  • preserved the existing row_condition behavior otherwise

Testing

python -m pytest tests\expectations\test_row_conditions.py -x -k "not spark"

Related Issue

Closes #11319

@netlify
Copy link
Copy Markdown

netlify bot commented Mar 24, 2026

‼️ Deploy request for niobium-lead-7998 rejected.

Name Link
🔨 Latest commit 8b61fc1

@gx-cla-bot
Copy link
Copy Markdown

gx-cla-bot bot commented Mar 24, 2026

A new contributor, HUZZAH! Welcome and thanks for joining our community. In order to accept a pull request we require that all contributors sign our Contributor License Agreement. We have two different CLAs, depending on whether you are contributing to GX in a personal or professional capacity. Please sign the one that is applicable to your situation so that we may accept your contribution:

Individual Contributor License Agreement v1.0
Software Grant and Corporate Contributor License Agreement v1.0

Once you have signed the CLA, you can add a comment with the text @cla-bot check and the bot will update the PR status!

Please reach out to the #gx-community-support channel, on our Slack if you have any questions or if you have already signed the CLA and are receiving this message in error.

Users missing a CLA: mallikramassani@gmail.com

@gx-cla-bot
Copy link
Copy Markdown

gx-cla-bot bot commented Mar 24, 2026

A new contributor, HUZZAH! Welcome and thanks for joining our community. In order to accept a pull request we require that all contributors sign our Contributor License Agreement. We have two different CLAs, depending on whether you are contributing to GX in a personal or professional capacity. Please sign the one that is applicable to your situation so that we may accept your contribution:

Individual Contributor License Agreement v1.0
Software Grant and Corporate Contributor License Agreement v1.0

Once you have signed the CLA, you can add a comment with the text @cla-bot check and the bot will update the PR status!

Please reach out to the #gx-community-support channel, on our Slack if you have any questions or if you have already signed the CLA and are receiving this message in error.

Users missing a CLA: mallikramassani@gmail.com

@Mallik544
Copy link
Copy Markdown
Author

Hi maintainers,

It looks like CI is failing at the check-actor-permissions step before test jobs run.

I’ve validated the change locally:
python -m pytest tests\expectations\test_row_conditions.py -x -k "not spark"

Could someone please re-run or approve the workflow so the full CI can execute?

Thanks!

@Mallik544
Copy link
Copy Markdown
Author

@cla-bot check

@gx-cla-bot
Copy link
Copy Markdown

gx-cla-bot bot commented Mar 24, 2026

A new contributor, HUZZAH! Welcome and thanks for joining our community. In order to accept a pull request we require that all contributors sign our Contributor License Agreement. We have two different CLAs, depending on whether you are contributing to GX in a personal or professional capacity. Please sign the one that is applicable to your situation so that we may accept your contribution:

Individual Contributor License Agreement v1.0
Software Grant and Corporate Contributor License Agreement v1.0

Once you have signed the CLA, you can add a comment with the text @cla-bot check and the bot will update the PR status!

Please reach out to the #gx-community-support channel, on our Slack if you have any questions or if you have already signed the CLA and are receiving this message in error.

Users missing a CLA: mallikramassani@gmail.com

@joshua-stauffer joshua-stauffer changed the title [BUGFIX] Fix row_condition handling for SQLAlchemyExecutionEngine [CONTRIB] Fix row_condition handling for SQLAlchemyExecutionEngine Mar 24, 2026
@joshua-stauffer
Copy link
Copy Markdown
Member

@cla-bot check

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 24, 2026

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 84.46%. Comparing base (8823228) to head (bfdd453).
⚠️ Report is 1 commits behind head on develop.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...ns/execution_engine/sqlalchemy_execution_engine.py 75.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #11742      +/-   ##
===========================================
- Coverage    84.59%   84.46%   -0.13%     
===========================================
  Files          473      473              
  Lines        39260    39262       +2     
===========================================
- Hits         33211    33162      -49     
- Misses        6049     6100      +51     
Flag Coverage Δ
3.10 73.50% <0.00%> (-0.01%) ⬇️
3.10 athena ?
3.10 aws_deps ?
3.10 big ?
3.10 clickhouse ?
3.10 filesystem ?
3.10 mysql ?
3.10 openpyxl or pyarrow or project or sqlite or aws_creds ?
3.10 postgresql ?
3.10 spark ?
3.10 spark_connect ?
3.10 sql_server ?
3.10 trino ?
3.11 ?
3.11 athena ?
3.11 aws_deps ?
3.11 big ?
3.11 clickhouse ?
3.11 filesystem ?
3.11 mysql ?
3.11 openpyxl or pyarrow or project or sqlite or aws_creds ?
3.11 postgresql ?
3.11 spark_connect ?
3.11 sql_server ?
3.11 trino ?
3.12 73.54% <0.00%> (-0.02%) ⬇️
3.12 athena ?
3.12 aws_deps ?
3.12 big ?
3.12 filesystem ?
3.12 mysql ?
3.12 openpyxl or pyarrow or project or sqlite or aws_creds ?
3.12 postgresql ?
3.12 spark ?
3.12 spark_connect ?
3.12 sql_server ?
3.12 trino ?
3.13 73.55% <0.00%> (-0.01%) ⬇️
3.13 athena 41.95% <0.00%> (-0.01%) ⬇️
3.13 aws_deps 45.20% <0.00%> (-0.01%) ⬇️
3.13 big 55.25% <0.00%> (-0.01%) ⬇️
3.13 bigquery 51.25% <50.00%> (-0.01%) ⬇️
3.13 clickhouse 41.96% <0.00%> (-0.01%) ⬇️
3.13 databricks ?
3.13 filesystem 64.33% <0.00%> (-0.01%) ⬇️
3.13 gx-redshift 51.40% <50.00%> (-0.01%) ⬇️
3.13 mysql 51.80% <75.00%> (-0.01%) ⬇️
3.13 openpyxl or pyarrow or project or sqlite or aws_creds 59.94% <75.00%> (-0.01%) ⬇️
3.13 postgresql 55.19% <75.00%> (-0.01%) ⬇️
3.13 snowflake 53.88% <50.00%> (+<0.01%) ⬆️
3.13 spark 55.89% <0.00%> (-0.01%) ⬇️
3.13 spark_connect 46.86% <0.00%> (-0.01%) ⬇️
3.13 sql_server 53.22% <50.00%> (-0.01%) ⬇️
3.13 trino 48.73% <0.00%> (-0.01%) ⬇️
cloud 0.00% <0.00%> (ø)
docs-basic 59.23% <0.00%> (-0.01%) ⬇️
docs-creds-needed 57.81% <0.00%> (-0.01%) ⬇️
docs-spark 57.28% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Member

@joshua-stauffer joshua-stauffer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the PR, @Mallik544 . Looks like we need a test case - the original issue mentions postgres, so that's probably the most straightforward candidate. I'd suggest using the parameterize_batch_for_data_sources fixture with PostgreSQLDatasourceTestConfig in the tests/integration/data_sources_and_expectations package.

@joshua-stauffer
Copy link
Copy Markdown
Member

Note that the original issue used the legacy RowCondition syntax with an illegal format. The new RowCondition syntax does allow logical operators, so this may or may not still be a bug. Use the current RowCondition API format in the test, and it should fail without your fix: https://docs.greatexpectations.io/docs/core/customize_expectations/row_conditions/#create-an-expectation-with-row-conditions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

row_condition not applied correctly with PostgreSQL/SQL datasources

3 participants