Array overlap to allow numeric operand#15964
Array overlap to allow numeric operand#15964sreemanamala wants to merge 10 commits intoapache:masterfrom
Conversation
| OperandTypes.family(SqlTypeFamily.STRING), | ||
| OperandTypes.family(SqlTypeFamily.NUMERIC) |
There was a problem hiding this comment.
it seems like its possible to possible to specify operands with different types
could you add some tests like:
SELECT dim3 FROM druid.numfoo WHERE ARRAY_OVERLAP(l1||'.1', ARRAY[2.1, 7.1]) LIMIT 5
SELECT dim3 FROM druid.numfoo WHERE ARRAY_OVERLAP(l1||'.1', ARRAY[2.1, 7.1]) LIMIT 5
SELECT dim3 FROM druid.numfoo WHERE ARRAY_OVERLAP(ARRAY[l2], ARRAY[2, 7]) LIMIT 5
SELECT dim3 FROM druid.numfoo WHERE ARRAY_OVERLAP(ARRAY['x',l2||''], ARRAY['2', '7']) LIMIT 5
SELECT dim3 FROM druid.numfoo WHERE ARRAY_OVERLAP(ARRAY[2,7], ARRAY['2', '7']) LIMIT 5
SELECT dim3 FROM druid.numfoo WHERE ARRAY_OVERLAP(ARRAY[2,7], ARRAY[2.0,7.0]) LIMIT 5
it seems like type mismatch was already there in some cases (array<int>,array<string>) - would it be possible to reject invalid cases?
There was a problem hiding this comment.
These are being taken care of in this PR
But rather than rejecting them, we would cast them properly and return correct response
There was a problem hiding this comment.
I don't explicitly see these cases being covered in that PR (or I missed it).
Can we cover them somehow?
sreemanamala
changed the title
| } else if (element instanceof String) { | ||
| node = SqlLiteral.createCharString((String) element, SqlParserPos.ZERO); | ||
| } else if (element instanceof Integer || element instanceof Long) { | ||
| } else if (element instanceof Integer || element instanceof Long || element instanceof Double) { |
There was a problem hiding this comment.
hmm, createExactNumeric doesn't really seem appropriate for Double, shouldn't that be handled with a separate else case, something like
...
} else if (element instanceof Float || element instanceof Double) {
node = SqlLiteral.createApproxNumeric(element.toString(), SqlParserPos.ZERO);
}
...
There was a problem hiding this comment.
Even I thought so. But little worried about in filter converting them to Strings. Havent completely gone through it. Let me check and change it to approx numeric.
…5920) * MSQ: Validate that strings and string arrays are not mixed. When multi-value strings and string arrays coexist in the same column, it causes problems with "classic MVD" style queries such as: select * from wikipedia -- fails at runtime select count(*) from wikipedia where flags = 'B' -- fails at planning time select flags, count(*) from wikipedia group by 1 -- fails at runtime To avoid these problems, this patch adds type verification for INSERT and REPLACE. It is targeted: the only type changes that are blocked are string-to-array and array-to-string. There is also a way to exclude certain columns from the type checks, if the user really knows what they're doing. * Fixes. * Tests and docs and error messages. * More docs. * Adjustments. * Adjust message. * Fix tests. * Fix test in DV mode.
|
i think the only hesitation i have here is it feels odd that this is the only array function that allows numeric inputs, i wonder if we should just allow them all to accept any input since at the native layer we allow the array functions to work with anything, if it isn't already and array we make it a single element array. This is largely done to make stuff work with schema evolution, and also mvds fit this pattern, but SQL is currently more restrictive.. and probably shouldn't have allowed VARCHAR either if we werent going to allow ANY to be symmetrical with the native layer. Need to think about it a bit. |
kgyrtkirk
left a comment
kgyrtkirk
left a comment
There was a problem hiding this comment.
could you please merge master - to get a fresh ci run?
| OperandTypes.family(SqlTypeFamily.STRING), | ||
| OperandTypes.family(SqlTypeFamily.NUMERIC) |
There was a problem hiding this comment.
I don't explicitly see these cases being covered in that PR (or I missed it).
Can we cover them somehow?
|
IMO, rather than expanding the |
4 participants
Description
Allow ARRAY_OVERLAP operator to take numeric value as input
Allow null & double values as elements of array passed as sql parameter
Release note
Key changed/added classes in this PR
ArrayOverlapOperatorConversionSqlParameterizerShuttleCalciteArraysQueryTestThis PR has: