TMUDF fixes and related changes #882
Conversation
A simple join query resulted in no plan. One of the reasons is the way we handle operator types for TMUDFs. Changed this so that the operator type indicates the arity of the operator.
This was caused by methods to generate an SQL string literal from a constant value. In some cases it repeated the character set name introducer.
Adding support for these new data types in TMUDFs. [TRAFODION-2392] Avoid costly sort in reducer TMUDFs Adding a new function type for TMUDFs that can avoid a costly sort for highly reducing UDFs that implement an internal hash table (or equivalent).
Wrong results were returned for passthru columns in TMUDFs when we had equal predicates on the source table. This was related to VEG rewrite, where the rewritten expression had the wrong type.
|
Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/1477/ |
|
Test Passed. https://jenkins.esgyn.com/job/Check-PR-master/1477/ |
| @@ -1027,7 +1052,7 @@ OperatorTypeEnum PredefinedTableMappingFunction::nameIsAPredefinedTMF(const Corr | |||
| const NAString &funcName = qualName.getObjectName(); | |||
There was a problem hiding this comment.
This comment is for line 1047. It must be the smallest nit recorded. The comment could be changed to REL_ANY_TABLE_MAPPING_UDF from REL_TABLE_MAPPING_UDF.
There was a problem hiding this comment.
Maybe small, but still a valid point! I stumbled over this while re-reading the code. Fixed.
| @@ -362,7 +362,7 @@ LmExprResult CreateLmOutputExpr(const NAType &formalType, | |||
| // specified type t to be converted to/from C strings. The only | |||
| // SQL types that do not need to be converted to C strings are: | |||
| // | |||
| // INT, SMALLINT, LARGEINT, FLOAT, REAL, DOUBLE PRECISION | |||
| // INT, SMALLINT, LARGEINT, FLOAT, REAL, DOUBLE PRECISION, BOOLEAN | |||
There was a problem hiding this comment.
Does Tinyint belong to this set of types? I suppose the answer is No, but thought it safer to check.
There was a problem hiding this comment.
Yes, good point, I'll add tinyint.
| throw new UDRException( | ||
| 38900, | ||
| "Overflow/underflow when assigning to TINYINT UNSIGNED type"); | ||
| else |
There was a problem hiding this comment.
Will this handle underflow correctly? If val < 0 is an exception supposed to be thrown?
There was a problem hiding this comment.
Thanks for finding this! Fixed in all three cases.
| // use the signed value that has the same bit | ||
| // pattern as the desired unsigned value, since | ||
| // Java doesn't support "unsigned" basic types | ||
| val = val + Byte.MIN_VALUE + Byte.MIN_VALUE; |
There was a problem hiding this comment.
I suppose this is basic arithmetic. If the comment could please include an example that will help. Is Byte.MIN_VALUE = -127. If yes, how will adding it twice to val, give the same bit pattern as the unsigned type? I am quite sure there is no defect here, but that I am not understanding something obvious. Thanks.
Fixed issues pointed out during the review and also searched the code for related issues (which existed). Fixed another problem, that interval data types don't use 1 byte integers, unlike numerics.
|
New Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/1488/ |
|
Test Passed. https://jenkins.esgyn.com/job/Check-PR-master/1488/ |
This PR has fixes for 5 JIRAs in a set of 4 commits (sorry, two of the fixes had overlaps, so I bundled them into a single commit). It may be easier to review each commit individually, since they are logically independent.
Commit 1:
[TRAFODION-2382] No plan produced when joining two TMUDFs
A simple join query resulted in no plan. One of the reasons is
the way we handle operator types for TMUDFs. Changed this so that
the operator type indicates the arity of the operator.
Commit 2:
[TRAFODION-2399] Syntax error when loading from salted table
This was caused by methods to generate an SQL string literal
from a constant value. In some cases it repeated the character
set name introducer.
Commit 3:
[TRAFODION-2383] Support tinyint and boolean in tmudfs
Adding support for these new data types in TMUDFs.
TRAFODION-2392 avoid costly sort in reducer TMUDFs
Add a REDUCER_NC function type. The only difference is in the
createContextForAChild function, where we don't generate a required
arrangement for UDFs of this type.
Misc changes, not related to a JIRA:
Commit 4:
[TRAFODION-2400] wrong results for passthru cols
Wrong results were returned for passthru columns in TMUDFs
when we had equal predicates on the source table. This was
related to VEG rewrite, where the rewritten expression had
the wrong type.
Add a cast node when the type of an expression changes after VEG rewrite