[CALCITE-7428] Support regexp function change regexp operator for Hive library#4818
[CALCITE-7428] Support regexp function change regexp operator for Hive library#4818cjj2010 wants to merge 5 commits intoapache:mainfrom
Conversation
|
There is a question in JIRA. The documentation you link to shows an infix operator REGEXP, but you are implementing support for a function. |
| case TRIM: | ||
| RelToSqlConverterUtil.unparseHiveTrim(writer, call, leftPrec, rightPrec); | ||
| break; | ||
| case REGEXP: |
There was a problem hiding this comment.
Judging from your Jira information, you want to add a new function, right? If you add this function, the dialect won't need to be modified.
There was a problem hiding this comment.
Judging from your Jira information, you want to add a new function, right? If you add this function, the dialect won't need to be modified.
Yes, REGEXP is an infix operator in Hive, but there is already a REGEXP function in Cacltie. If another REGEXP operator is added, the original SQL parsing will report an error: "Incorrect syntax near the keyword 'REGEXP'". Therefore, my idea is to convert the REGEXP function into a REGEXP operator based on the Hive dialect, I'm not sure if this is correct
There was a problem hiding this comment.
Based on your Jira description, is it true that select brand_name from product where REGEXP(brand_name,'[a-zA-Z]') won't work in Hive? It needs to be converted to select brand_name from product where brand_name REGEXP '[a-zA-Z]'.
There was a problem hiding this comment.
Based on your Jira description, is it true that
select brand_name from product where REGEXP(brand_name,'[a-zA-Z]')won't work in Hive? It needs to be converted toselect brand_name from product where brand_name REGEXP '[a-zA-Z]'.
Yes
There was a problem hiding this comment.
Okay, please describe it in detail in Jira, it doesn't seem very clear.
There was a problem hiding this comment.
Okay, please describe it in detail in Jira, it doesn't seem very clear.
Thank you for your suggestion. The changes have been made more accurately
There was a problem hiding this comment.
Another point is that, as I understand it from Jira's perspective, there's no need to introduce a new SqlKind.
| OTHER_DDL, | ||
|
|
||
| /** The {@code REGEXP} function. */ | ||
| REGEXP; |
There was a problem hiding this comment.
Why make this change? If it were a dialect conversion, it could be determined using SqlOperator.
There was a problem hiding this comment.
Why make this change? If it were a dialect conversion, it could be determined using
SqlOperator.
Can we modify the function to
SqlBasicFunction.create(SqlKind.RLIKE, ReturnTypes.BOOLEAN_NULLABLE,
OperandTypes.STRING_STRING);
Using SQL Kind.RLIKE, it seems that there is a need for a kind in HiveSQL Dialect to perform conversion judgments, and I am not sure if I understand it correctly
There was a problem hiding this comment.
I think you can refer to the suggestions in Jira.
There was a problem hiding this comment.
I think you can refer to the suggestions in Jira.
I have already modified and resubmitted the code. Can you help me review the code again. Thank you
| case TRIM: | ||
| RelToSqlConverterUtil.unparseHiveTrim(writer, call, leftPrec, rightPrec); | ||
| break; | ||
| case REGEXP: |
There was a problem hiding this comment.
Another point is that, as I understand it from Jira's perspective, there's no need to introduce a new SqlKind.
| */ | ||
| public static void unparseRegexp(SqlWriter writer, SqlCall call, int leftPrec, int rightPrec) { | ||
| if (call.operandCount() != 2) { | ||
| throw new IllegalArgumentException("REGEXP operator requires exactly 2 operands"); |
There was a problem hiding this comment.
This line of code was not covered by tests.
There was a problem hiding this comment.
This line of code was not covered by tests.
I tried to execute REGEXP ("brandname"), which checks for errors during the SQL parsing validation phase and does not enter the dialect parsing phase. I think I need to remove the judgment logic in the code
|
|
||
| /** The "REGEXP(value, regexp)" function, equivalent to {@link #RLIKE}. */ | ||
| @LibraryOperator(libraries = {SPARK}) | ||
| @LibraryOperator(libraries = {SPARK, HIVE}) |
There was a problem hiding this comment.
Since it's been added to libraries in this PR, Hive should also be added to SqlOperatorTest.
There was a problem hiding this comment.
Since it's been added to libraries in this PR, Hive should also be added to SqlOperatorTest.
done
cjj2010
changed the title
|
3 participants
jira: https://issues.apache.org/jira/browse/CALCITE-7428