Allow complex expression rewrites with approximate numeric constant #21892
Conversation
|
WIll update once the test failures are fixed. |
Praveen2112
force-pushed
the
praveen/jdbc_complex_rewrite_2
branch
4 times, most recently
from
1ffef64
to
76c3f16
Compare
| try (TestTable testTable = new TestTable( | ||
| getQueryRunner()::execute, | ||
| "test_complex_expression_pushdown_", | ||
| "AS SELECT 1 as a_int, REAL '3.14' as a_real, DOUBLE '1.9891e30' as a_double")) { |
There was a problem hiding this comment.
add a test with NaN stored in the remote database (for systems that do support storing NaN)
and queries col < 1, col > 1, col > -inf, col < +inf.
There was a problem hiding this comment.
I'll address it as a follow-up PR
Praveen2112
force-pushed
the
praveen/jdbc_complex_rewrite_2
branch
2 times, most recently
from
5516763
to
c42ccec
Compare
Enable them for MariaDB, MySQL, Oracle and Ignite connectors
Praveen2112
force-pushed
the
praveen/jdbc_complex_rewrite_2
branch
from
c42ccec
to
c99fe17
Compare
Praveen2112
force-pushed
the
praveen/jdbc_complex_rewrite_2
branch
from
c99fe17
to
5f4033d
Compare
| Type type = constant.getType(); | ||
| Object value = constant.getValue(); | ||
| if (value == null) { | ||
| // TODO we could handle NULL values too |
There was a problem hiding this comment.
nit: should we add ticket number for the todo?
There was a problem hiding this comment.
NULL literal is rarely practical for pushdown, since most of the time this means the encompassing expression is NULL and should be simplified in the trino engine.
i am sure there are cases where it's different (like trino format function), but can't of anything non Trino specific yet.
| "complex_expression_", | ||
| "AS SELECT 1 as a_int, REAL '3.14' as a_real, DOUBLE '1.9891e30' as a_double")) { | ||
| assertThat(query("SELECT a_int FROM " + testTable.getName() + " WHERE a_real > REAL '3.12' OR a_double > DOUBLE '1.9890e30'")) | ||
| .isFullyPushedDown(); |
There was a problem hiding this comment.
can we add testcase with at least either Double.MIN_VALUE or Double.MIN_VALUE and Float.MIN_VALUE or Float.MIN_VALUE
There was a problem hiding this comment.
good point
with at least either Double.MIN_VALUE or Double.MIN_VALUE
not "or". all these should be tested
There was a problem hiding this comment.
MIN_VALUE or MAX_VALUE as a part of data or constant or both ?
Description
Currently
RewriteExactNumericConstantdoesn't support rewriting expressions if they are ofDOUBLEorREALtype, so we are generalizing it toRewriteNumericConstant.Additional context and related issues
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:
This would affect the JDBC connectors in general so should implement it for each of them or do we have a generic options for JDBC ?