-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Reconstruct NOT IN when pushing down to JDBC source #6343
Conversation
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.
% comments
"AND ((\"col_7\" >= ? AND \"col_7\" < ?) OR (\"col_7\" >= ? AND \"col_7\" < ?)) " + | ||
"AND ((\"col_8\" >= ? AND \"col_8\" < ?) OR (\"col_8\" >= ? AND \"col_8\" <= ?)) " + | ||
"AND ((\"col_9\" < ?) OR \"col_9\" IN (?,?)) " + | ||
"AND (\"col_2\" = ?)"); |
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.
I don't like this assertion it is very detailed and so it it will be fragile to changes
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.
It is required to visualize the changes and this is the only place where we test queries output to remote system.
Also, the structure of output queries changes very very rarely, so expected maint cost is low.
@@ -31,10 +33,13 @@ private Utils() {} | |||
// just for this use case. | |||
static final TypeOperators TUPLE_DOMAIN_TYPE_OPERATORS = new TypeOperators(); | |||
|
|||
public static Block nativeValueToBlock(Type type, Object object) | |||
public static Block nativeValueToBlock(Type type, @Nullable Object object) |
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.
Optional (separate commit)?
@@ -290,8 +290,8 @@ public void testBuildSqlWithDomainComplement() | |||
assertThat(lastQuery).isEqualTo("" + |
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.
Do we have query in ATDQ to trigger this pushdown?
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.
AC |
Fixes #6075