You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "xuzifu666 (via GitHub)" <gi...@apache.org> on 2025/01/24 03:11:30 UTC
[PR] [CALCITE-6241] Enable a few existing functions to hive library [calcite]
xuzifu666 opened a new pull request, #4165:
URL: https://github.com/apache/calcite/pull/4165
Add hive as a supported library for functions that have already been implemented for other libraries.
issue: https://issues.apache.org/jira/browse/CALCITE-6241 and commucate with author make a new pr to finish it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6800] Enable a few existing functions to HIVE library [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4165:
URL: https://github.com/apache/calcite/pull/4165#issuecomment-2623454481
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4165) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4165&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4165&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4165&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4165&metric=new_coverage&view=list)
 [20.9% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4165&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4165)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6800] Enable a few existing functions to HIVE library [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on PR #4165:
URL: https://github.com/apache/calcite/pull/4165#issuecomment-2624331518
> @xuzifu666 sorry, but IIANM the list of functions in the description of the Jira/PR is still inaccurate: it mentions `INSTR` function (and it shouldn't, since this function was already defined for Hive); and it does not mention the `SPACE` function (and it should, since the changes consider it). Basically `INSTR` should be replaced by `SPACE` in the list, or am I missing something?
Oh, it's my mistake, very sorry and I remove INSTR and add SPACE to description of JIRA and pr, very thanks for your review.@rubenada
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6241] Enable a few existing functions to hive library [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on code in PR #4165:
URL: https://github.com/apache/calcite/pull/4165#discussion_r1928134460
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -134,7 +134,7 @@ private SqlLibraryOperators() {
/** The "DATE_ADD(date, numDays)" function
* (Spark) Returns the date that is num_days after start_date. */
- @LibraryOperator(libraries = {SPARK})
+ @LibraryOperator(libraries = {SPARK, ORACLE, HIVE})
Review Comment:
delete oracle~ it is my mistake.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6800] Enable a few existing functions to HIVE library [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on PR #4165:
URL: https://github.com/apache/calcite/pull/4165#issuecomment-2623755209
@xuzifu666 sorry, but IIANM the list of functions in the description of the Jira/PR is still inaccurate: it mentions `INSTR` function (and it shouldn't, since this function was already defined for Hive); and it does not mention the `SPACE` function (and it should, since the changes consider it). Basically `INSTR` should be replaced by `SPACE` in the list, or am I missing something?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6800] Enable a few existing functions to HIVE library [calcite]
Posted by "ILuffZhe (via GitHub)" <gi...@apache.org>.
ILuffZhe commented on PR #4165:
URL: https://github.com/apache/calcite/pull/4165#issuecomment-2623345933
> > > > It seems your changes in PR is not consistent with what described in JIRA.
> > >
> > >
> > > This pr is aim to enable some existing functions to Hive library, I had changed purpose about it. Would you give some advice if it is not fitable. Thanks~ @ILuffZhe
> >
> >
> > This case should only enable existing functions which have the same behavior in Hive.
> > If not, maybe we should file a new JIRA ticket and implement it if you want.
>
> This case is only enable existing functions which have the same behavior in Hive，not implement for new functions，so keep it can be fine without file a new jira ticket or can merge this pr firstly? @ILuffZhe
Sorry for the delay, been a little busy these days.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6241] Enable a few existing functions to hive library [calcite]
Posted by "ILuffZhe (via GitHub)" <gi...@apache.org>.
ILuffZhe commented on PR #4165:
URL: https://github.com/apache/calcite/pull/4165#issuecomment-2611706236
@xuzifu666 The CI failed ,please make sure your change can pass on your machine, then push it so we can go forward.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6241] Enable a few existing functions to hive library [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on PR #4165:
URL: https://github.com/apache/calcite/pull/4165#issuecomment-2612312785
> @xuzifu666 The CI failed ,please make sure your change can pass on your machine, then push it so we can go forward.
@ILuffZhe CI is success, PTAL when you are free time, thanks~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6241] Enable a few existing functions to hive library [calcite]
Posted by "ILuffZhe (via GitHub)" <gi...@apache.org>.
ILuffZhe commented on code in PR #4165:
URL: https://github.com/apache/calcite/pull/4165#discussion_r1929539588
##########
site/_docs/reference.md:
##########
@@ -2852,7 +2852,7 @@ In the following:
| b | DATE(string) | Equivalent to `CAST(string AS DATE)`
| b | DATE(year, month, day) | Returns a DATE value for *year*, *month*, and *day* (all of type INTEGER)
| q r f | DATEADD(timeUnit, integer, datetime) | Equivalent to `TIMESTAMPADD(timeUnit, integer, datetime)`
-| q r f | DATEDIFF(timeUnit, datetime, datetime2) | Equivalent to `TIMESTAMPDIFF(timeUnit, datetime, datetime2)`
+| q r f h | DATEDIFF(timeUnit, datetime, datetime2) | Equivalent to `TIMESTAMPDIFF(timeUnit, datetime, datetime2)`
Review Comment:
Is this a unintentional mistake?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6241] Enable a few existing functions to hive library [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on code in PR #4165:
URL: https://github.com/apache/calcite/pull/4165#discussion_r1928137340
##########
site/_docs/reference.md:
##########
@@ -2780,7 +2780,7 @@ In the following:
| m | expr1 <=> expr2 | Whether two values are equal, treating null values as the same, and it's similar to `IS NOT DISTINCT FROM`
| p | ACOSD(numeric) | Returns the inverse cosine of *numeric* in degrees as a double. Returns NaN if *numeric* is NaN. Fails if *numeric* is less than -1.0 or greater than 1.0.
| * | ACOSH(numeric) | Returns the inverse hyperbolic cosine of *numeric*
-| o s | ADD_MONTHS(date, numMonths) | Returns the date that is *numMonths* after *date*
+| o s h | ADD_MONTHS(date, numMonths) | Returns the date that is *numMonths* after *date*
Review Comment:
Had remove this function.
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -134,7 +134,7 @@ private SqlLibraryOperators() {
/** The "DATE_ADD(date, numDays)" function
* (Spark) Returns the date that is num_days after start_date. */
- @LibraryOperator(libraries = {SPARK})
+ @LibraryOperator(libraries = {SPARK, ORACLE, HIVE})
Review Comment:
remove oracle~ it is my mistake.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6241] Enable a few existing functions to hive library [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on code in PR #4165:
URL: https://github.com/apache/calcite/pull/4165#discussion_r1928134082
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -134,7 +134,7 @@ private SqlLibraryOperators() {
/** The "DATE_ADD(date, numDays)" function
* (Spark) Returns the date that is num_days after start_date. */
- @LibraryOperator(libraries = {SPARK})
+ @LibraryOperator(libraries = {SPARK, ORACLE, HIVE})
Review Comment:
Thanks for reminder, hive not support both, only support DATE_ADD(date, numDays), I would change it. @ILuffZhe
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6800] Enable a few existing functions to HIVE library [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #4165:
URL: https://github.com/apache/calcite/pull/4165#issuecomment-2626111754
@rubenada @ILuffZhe thanks for your review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6800] Enable a few existing functions to HIVE library [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4165:
URL: https://github.com/apache/calcite/pull/4165#issuecomment-2614008749
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4165) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4165&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4165&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4165&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4165&metric=new_coverage&view=list)
 [20.5% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4165&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4165)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6241] Enable a few existing functions to hive library [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4165:
URL: https://github.com/apache/calcite/pull/4165#issuecomment-2613944785
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4165) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4165&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4165&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4165&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4165&metric=new_coverage&view=list)
 [18.9% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4165&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4165)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6241] Enable a few existing functions to hive library [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on code in PR #4165:
URL: https://github.com/apache/calcite/pull/4165#discussion_r1929528581
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -162,7 +162,7 @@ private SqlLibraryOperators() {
* <p>MySQL has "DATEDIFF(date, date2)" and "TIMEDIFF(time, time2)" functions
* but Calcite does not implement these because they have no "timeUnit"
* argument. */
- @LibraryOperator(libraries = {MSSQL, REDSHIFT, SNOWFLAKE})
+ @LibraryOperator(libraries = {MSSQL, REDSHIFT, SNOWFLAKE, HIVE})
Review Comment:
date_diff is not compative，and had test for date_sub and repeat function @ILuffZhe
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6241] Enable a few existing functions to hive library [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on code in PR #4165:
URL: https://github.com/apache/calcite/pull/4165#discussion_r1929528463
##########
site/_docs/reference.md:
##########
@@ -2888,7 +2888,7 @@ In the following:
| b | FORMAT_TIME(string, time) | Formats *time* according to the specified format *string*
| b | FORMAT_TIMESTAMP(string timestamp) | Formats *timestamp* according to the specified format *string*
| s | GETBIT(value, position) | Equivalent to `BIT_GET(value, position)`
-| b o p r s | GREATEST(expr [, expr ]*) | Returns the greatest of the expressions
+| b o p r s h | GREATEST(expr [, expr ]*) | Returns the greatest of the expressions
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6241] Enable a few existing functions to hive library [calcite]
Posted by "ILuffZhe (via GitHub)" <gi...@apache.org>.
ILuffZhe commented on PR #4165:
URL: https://github.com/apache/calcite/pull/4165#issuecomment-2613965644
Hi @xuzifu666 , you don't need to force-push every time, which makes it not easier to review.
In this case, make sure all the functions enabled in Hive library is document in reference.md, and SqlOperatorTest should also cover those functions (the last commit still missed some tests).
Please take a look and cc me when you are ready.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6800] Enable a few existing functions to HIVE library [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on PR #4165:
URL: https://github.com/apache/calcite/pull/4165#issuecomment-2623434837
> Overall looks good, one minor comment:
>
> * `SPACE` is missing from the list of functions on the PR / Jira description, but it was added in the code
> * `INSTR` is part of the list; however this function was already defined for Hive
@rubenada Thanks for your reminder, 2 is fixed now, and 1 I had added a space in the funtion list of PR / Jira description.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6800] Enable a few existing functions to HIVE library [calcite]
Posted by "ILuffZhe (via GitHub)" <gi...@apache.org>.
ILuffZhe commented on PR #4165:
URL: https://github.com/apache/calcite/pull/4165#issuecomment-2614288192
It seems your changes in PR is not consistent with what described in JIRA.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6800] Enable a few existing functions to hive library [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on PR #4165:
URL: https://github.com/apache/calcite/pull/4165#issuecomment-2613997498
> Hi @xuzifu666 , you don't need to force-push every time, which makes it not easier to review. In this case, make sure all the functions enabled in Hive library is document in reference.md, and SqlOperatorTest should also cover those functions (the last commit still missed some tests). Please take a look and cc me when you are ready.
>
> By the way, I think it might be better to add a new JIRA ticket, [CALCITE-6241](https://issues.apache.org/jira/browse/CALCITE-6241) is for Spark library.
Very thanks for your remind, I had add all changes in doc and add all cases in Test, PTAL when you free time. @ILuffZhe
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6800] Enable a few existing functions to HIVE library [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on code in PR #4165:
URL: https://github.com/apache/calcite/pull/4165#discussion_r1934370027
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -401,7 +401,7 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding operatorBinding,
public static final SqlFunction STRPOS = new SqlPositionFunction("STRPOS");
/** The "INSTR(string, substring [, position [, occurrence]])" function. */
- @LibraryOperator(libraries = {BIG_QUERY, HIVE, MYSQL, ORACLE})
+ @LibraryOperator(libraries = {BIG_QUERY, HIVE, MYSQL, ORACLE, HIVE})
Review Comment:
Hive was already in the list.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6800] Enable a few existing functions to HIVE library [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai merged PR #4165:
URL: https://github.com/apache/calcite/pull/4165
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6800] Enable a few existing functions to HIVE library [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on PR #4165:
URL: https://github.com/apache/calcite/pull/4165#issuecomment-2621363734
@rubenada could you give a final review，thanks~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6241] Enable a few existing functions to hive library [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on PR #4165:
URL: https://github.com/apache/calcite/pull/4165#issuecomment-2613945221
@ILuffZhe addressed，thanks for your review，PTAL
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6800] Enable a few existing functions to HIVE library [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on PR #4165:
URL: https://github.com/apache/calcite/pull/4165#issuecomment-2614308955
> > > It seems your changes in PR is not consistent with what described in JIRA.
> >
> > This pr is aim to enable some existing functions to Hive library, I had changed purpose about it. Would you give some advice if it is not fitable. Thanks~ @ILuffZhe
>
> This case should only enable existing functions which have the same behavior in Hive.
> If not, maybe we should file a new JIRA ticket and implement it if you want.
This case is only enable existing functions which have the same behavior in Hive，not implement for new function @ILuffZhe
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6241] Enable a few existing functions to hive library [calcite]
Posted by "ILuffZhe (via GitHub)" <gi...@apache.org>.
ILuffZhe commented on code in PR #4165:
URL: https://github.com/apache/calcite/pull/4165#discussion_r1929449604
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -162,7 +162,7 @@ private SqlLibraryOperators() {
* <p>MySQL has "DATEDIFF(date, date2)" and "TIMEDIFF(time, time2)" functions
* but Calcite does not implement these because they have no "timeUnit"
* argument. */
- @LibraryOperator(libraries = {MSSQL, REDSHIFT, SNOWFLAKE})
+ @LibraryOperator(libraries = {MSSQL, REDSHIFT, SNOWFLAKE, HIVE})
Review Comment:
testDateDiff() and testDateSub()
##########
site/_docs/reference.md:
##########
@@ -2888,7 +2888,7 @@ In the following:
| b | FORMAT_TIME(string, time) | Formats *time* according to the specified format *string*
| b | FORMAT_TIMESTAMP(string timestamp) | Formats *timestamp* according to the specified format *string*
| s | GETBIT(value, position) | Equivalent to `BIT_GET(value, position)`
-| b o p r s | GREATEST(expr [, expr ]*) | Returns the greatest of the expressions
+| b o p r s h | GREATEST(expr [, expr ]*) | Returns the greatest of the expressions
Review Comment:
so is testGreatestFunc()
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6800] Enable a few existing functions to hive library [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on code in PR #4165:
URL: https://github.com/apache/calcite/pull/4165#discussion_r1929560327
##########
site/_docs/reference.md:
##########
@@ -2852,7 +2852,7 @@ In the following:
| b | DATE(string) | Equivalent to `CAST(string AS DATE)`
| b | DATE(year, month, day) | Returns a DATE value for *year*, *month*, and *day* (all of type INTEGER)
| q r f | DATEADD(timeUnit, integer, datetime) | Equivalent to `TIMESTAMPADD(timeUnit, integer, datetime)`
-| q r f | DATEDIFF(timeUnit, datetime, datetime2) | Equivalent to `TIMESTAMPDIFF(timeUnit, datetime, datetime2)`
+| q r f h | DATEDIFF(timeUnit, datetime, datetime2) | Equivalent to `TIMESTAMPDIFF(timeUnit, datetime, datetime2)`
Review Comment:
Had removed it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6800] Enable a few existing functions to HIVE library [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on PR #4165:
URL: https://github.com/apache/calcite/pull/4165#issuecomment-2614295028
> It seems your changes in PR is not consistent with what described in JIRA.
This pr is want to enable some existing functions to Hive library, I had changed purpose about it. Would you give some advice if it is not fitable. Thanks~ @ILuffZhe
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6241] Enable a few existing functions to hive library [calcite]
Posted by "ILuffZhe (via GitHub)" <gi...@apache.org>.
ILuffZhe commented on code in PR #4165:
URL: https://github.com/apache/calcite/pull/4165#discussion_r1929449604
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -162,7 +162,7 @@ private SqlLibraryOperators() {
* <p>MySQL has "DATEDIFF(date, date2)" and "TIMEDIFF(time, time2)" functions
* but Calcite does not implement these because they have no "timeUnit"
* argument. */
- @LibraryOperator(libraries = {MSSQL, REDSHIFT, SNOWFLAKE})
+ @LibraryOperator(libraries = {MSSQL, REDSHIFT, SNOWFLAKE, HIVE})
Review Comment:
testDateDiff(), testDateSub() and testRepeatFunc()
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6800] Enable a few existing functions to HIVE library [calcite]
Posted by "rubenada (via GitHub)" <gi...@apache.org>.
rubenada commented on PR #4165:
URL: https://github.com/apache/calcite/pull/4165#issuecomment-2622514812
Overall looks good, one minor comment:
- `SPACE` is missing from the list of functions on the PR / Jira description, but it was added in the code
- `INSTR` is part of the list; however this function was already defined for Hive
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6241] Enable a few existing functions to hive library [calcite]
Posted by "ILuffZhe (via GitHub)" <gi...@apache.org>.
ILuffZhe commented on code in PR #4165:
URL: https://github.com/apache/calcite/pull/4165#discussion_r1928121630
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -134,7 +134,7 @@ private SqlLibraryOperators() {
/** The "DATE_ADD(date, numDays)" function
* (Spark) Returns the date that is num_days after start_date. */
- @LibraryOperator(libraries = {SPARK})
+ @LibraryOperator(libraries = {SPARK, ORACLE, HIVE})
Review Comment:
You enable this in Oracle, do we have a test?
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -134,7 +134,7 @@ private SqlLibraryOperators() {
/** The "DATE_ADD(date, numDays)" function
* (Spark) Returns the date that is num_days after start_date. */
- @LibraryOperator(libraries = {SPARK})
+ @LibraryOperator(libraries = {SPARK, ORACLE, HIVE})
Review Comment:
Does hive support both `DATE_ADD(date, interval)` and `DATE_ADD(date, numDays)`?
##########
site/_docs/reference.md:
##########
@@ -2780,7 +2780,7 @@ In the following:
| m | expr1 <=> expr2 | Whether two values are equal, treating null values as the same, and it's similar to `IS NOT DISTINCT FROM`
| p | ACOSD(numeric) | Returns the inverse cosine of *numeric* in degrees as a double. Returns NaN if *numeric* is NaN. Fails if *numeric* is less than -1.0 or greater than 1.0.
| * | ACOSH(numeric) | Returns the inverse hyperbolic cosine of *numeric*
-| o s | ADD_MONTHS(date, numMonths) | Returns the date that is *numMonths* after *date*
+| o s h | ADD_MONTHS(date, numMonths) | Returns the date that is *numMonths* after *date*
Review Comment:
This function seems not being added above, please take a look and also make sure the reference's changing consistent with what in SqlLibraryOpetators.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6241] Enable a few existing functions to hive library [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4165:
URL: https://github.com/apache/calcite/pull/4165#issuecomment-2612168037
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4165) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4165&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4165&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4165&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4165&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4165&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4165)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6241] Enable a few existing functions to hive library [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4165:
URL: https://github.com/apache/calcite/pull/4165#issuecomment-2612295729
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4165) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4165&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4165&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4165&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4165&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4165&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4165)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6241] Enable a few existing functions to hive library [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 closed pull request #4165: [CALCITE-6241] Enable a few existing functions to hive library
URL: https://github.com/apache/calcite/pull/4165
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6800] Enable a few existing functions to HIVE library [calcite]
Posted by "ILuffZhe (via GitHub)" <gi...@apache.org>.
ILuffZhe commented on PR #4165:
URL: https://github.com/apache/calcite/pull/4165#issuecomment-2614301618
> > It seems your changes in PR is not consistent with what described in JIRA.
>
> This pr is aim to enable some existing functions to Hive library, I had changed purpose about it. Would you give some advice if it is not fitable. Thanks~ @ILuffZhe
This case should only enable existing functions which have the same behavior in Hive.
If not, maybe we should file a new JIRA ticket and implement it if you want.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6800] Enable a few existing functions to HIVE library [calcite]
Posted by "xuzifu666 (via GitHub)" <gi...@apache.org>.
xuzifu666 commented on PR #4165:
URL: https://github.com/apache/calcite/pull/4165#issuecomment-2615325287
@ILuffZhe Could you give a look again? Thanks~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org