-
Notifications
You must be signed in to change notification settings - Fork 16.6k
fix: allow null comparator in adhocfilter #16936
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
Conversation
|
/testenv up |
|
@suddjian Ephemeral environment spinning up at http://35.167.17.93:8080. Credentials are |
geido
left a comment
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.
LGTM! Thanks for the fix
Codecov Report
@@ Coverage Diff @@
## master #16936 +/- ##
==========================================
- Coverage 76.97% 76.93% -0.05%
==========================================
Files 1023 1030 +7
Lines 54907 55024 +117
Branches 7486 7466 -20
==========================================
+ Hits 42266 42333 +67
- Misses 12393 12437 +44
- Partials 248 254 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
zhaoyongjie
left a comment
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.
Please hold on this PR. the NULL in the select is serialized SQL NULL. So we should generate a SQL without NULL at here.
superset/superset/connectors/sqla/models.py
Lines 713 to 737 in c577191
| def values_for_column(self, column_name: str, limit: int = 10000) -> List[Any]: | |
| """Runs query against sqla to retrieve some | |
| sample values for the given column. | |
| """ | |
| cols = {col.column_name: col for col in self.columns} | |
| target_col = cols[column_name] | |
| tp = self.get_template_processor() | |
| qry = ( | |
| select([target_col.get_sqla_col()]) | |
| .select_from(self.get_from_clause(tp)) | |
| .distinct() | |
| ) | |
| if limit: | |
| qry = qry.limit(limit) | |
| if self.fetch_values_predicate: | |
| qry = qry.where(self.get_fetch_values_predicate()) | |
| engine = self.database.get_sqla_engine() | |
| sql = "{}".format(qry.compile(engine, compile_kwargs={"literal_binds": True})) | |
| sql = self.mutate_query_from_config(sql) | |
| df = pd.read_sql_query(sql=sql, con=engine) | |
| return df[column_name].to_list() |
|
Ephemeral environment shutdown and build artifacts deleted. |
SUMMARY
This pr fixes an issue where in the adhocfilter control null value is not allowed causing the user not able to save.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
after
Screen.Recording.2021-10-01.at.8.55.00.AM.mov
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION