You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "zoudan (via GitHub)" <gi...@apache.org> on 2023/03/23 16:08:15 UTC

[GitHub] [calcite] zoudan opened a new pull request, #3128: [CALCITE-5606] Add SqlLibrary.ALL and change the library of TANH, COSH, SINH to it

zoudan opened a new pull request, #3128:
URL: https://github.com/apache/calcite/pull/3128

   Add a new SqlLibrary named 'ALL' with abbreviation '*', if an operator is marked with this SqlLibrary, it means it's a common one and should belong to all the library. TANH, COSH, SINH are three of these operators.


-- 
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


[GitHub] [calcite] zoudan commented on a diff in pull request #3128: [CALCITE-5606] Add SqlLibrary.ALL and change the library of TANH, COSH, SINH to it

Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on code in PR #3128:
URL: https://github.com/apache/calcite/pull/3128#discussion_r1148567731


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibrary.java:
##########
@@ -94,11 +97,21 @@ public enum SqlLibrary {
   /** Parses a comma-separated string such as "standard,oracle". */
   public static List<SqlLibrary> parse(String libraryNameList) {
     final ImmutableList.Builder<SqlLibrary> list = ImmutableList.builder();
-    for (String libraryName : libraryNameList.split(",")) {
-      SqlLibrary library =
-          requireNonNull(SqlLibrary.of(libraryName),
-              () -> "library does not exist: " + libraryName);
-      list.add(library);
+    List<String> libList = Arrays.asList(libraryNameList.split(","));

Review Comment:
   I have changed this function, please have another look when you have time.



-- 
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


[GitHub] [calcite] julianhyde closed pull request #3128: [CALCITE-5606] Add SqlLibrary.ALL and change the library of TANH, COSH, SINH to it

Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde closed pull request #3128: [CALCITE-5606] Add SqlLibrary.ALL and change the library of TANH, COSH, SINH to it
URL: https://github.com/apache/calcite/pull/3128


-- 
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


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3128: [CALCITE-5606] Add SqlLibrary.ALL and change the library of TANH, COSH, SINH to it

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3128:
URL: https://github.com/apache/calcite/pull/3128#issuecomment-1482168896

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3128)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3128&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3128&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3128&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=CODE_SMELL)
   
   [![91.1%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '91.1%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3128&metric=new_coverage&view=list) [91.1% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3128&metric=new_coverage&view=list)  
   [![4.3%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/5-16px.png '4.3%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3128&metric=new_duplicated_lines_density&view=list) [4.3% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3128&metric=new_duplicated_lines_density&view=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


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3128: [CALCITE-5606] Add SqlLibrary.ALL and change the library of TANH, COSH, SINH to it

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3128:
URL: https://github.com/apache/calcite/pull/3128#issuecomment-1481499729

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3128)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3128&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3128&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3128&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=CODE_SMELL)
   
   [![90.7%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/90-16px.png '90.7%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3128&metric=new_coverage&view=list) [90.7% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3128&metric=new_coverage&view=list)  
   [![4.3%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/5-16px.png '4.3%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3128&metric=new_duplicated_lines_density&view=list) [4.3% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3128&metric=new_duplicated_lines_density&view=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


[GitHub] [calcite] zoudan commented on a diff in pull request #3128: [CALCITE-5606] Add SqlLibrary.ALL and change the library of TANH, COSH, SINH to it

Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on code in PR #3128:
URL: https://github.com/apache/calcite/pull/3128#discussion_r1147047473


##########
site/_docs/reference.md:
##########
@@ -2648,7 +2648,7 @@ BigQuery's type system uses confusingly different names for types and functions:
 | b | ARRAY_REVERSE(array)                           | Reverses elements of *array*
 | m s | CHAR(integer)                                | Returns the character whose ASCII code is *integer* % 256, or null if *integer* &lt; 0
 | o p | CHR(integer)                                 | Returns the character whose UTF-8 code is *integer*
-| b o | COSH(numeric)                                | Returns the hyperbolic cosine of *numeric*
+| * | COSH(numeric)                                  | Returns the hyperbolic cosine of *numeric*

Review Comment:
   thanks for reminding



-- 
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


[GitHub] [calcite] zoudan commented on pull request #3128: [CALCITE-5606] Add SqlLibrary.ALL and change the library of TANH, COSH, SINH to it

Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on PR #3128:
URL: https://github.com/apache/calcite/pull/3128#issuecomment-1482172357

   @tanclary I have updated this CR, please have a look when you have time.


-- 
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


[GitHub] [calcite] tanclary commented on a diff in pull request #3128: [CALCITE-5606] Add SqlLibrary.ALL and change the library of TANH, COSH, SINH to it

Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #3128:
URL: https://github.com/apache/calcite/pull/3128#discussion_r1146504287


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibrary.java:
##########
@@ -68,7 +69,9 @@ public enum SqlLibrary {
   POSTGRESQL("p", "postgresql"),
   /** A collection of operators that are in Apache Spark but not in standard
    * SQL. */
-  SPARK("s", "spark");
+  SPARK("s", "spark"),
+  /** A collection of operators that could be used in all libraries except STANDARD and SPATIAL. */
+  ALL("*", "all");

Review Comment:
   Should this be alphabetical?



##########
site/_docs/reference.md:
##########
@@ -2648,7 +2648,7 @@ BigQuery's type system uses confusingly different names for types and functions:
 | b | ARRAY_REVERSE(array)                           | Reverses elements of *array*
 | m s | CHAR(integer)                                | Returns the character whose ASCII code is *integer* % 256, or null if *integer* &lt; 0
 | o p | CHR(integer)                                 | Returns the character whose UTF-8 code is *integer*
-| b o | COSH(numeric)                                | Returns the hyperbolic cosine of *numeric*
+| * | COSH(numeric)                                  | Returns the hyperbolic cosine of *numeric*

Review Comment:
   I think it could be useful to have a line in reference.md that tells readers that * = all libraries.



##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibrary.java:
##########
@@ -94,11 +97,21 @@ public enum SqlLibrary {
   /** Parses a comma-separated string such as "standard,oracle". */
   public static List<SqlLibrary> parse(String libraryNameList) {
     final ImmutableList.Builder<SqlLibrary> list = ImmutableList.builder();
-    for (String libraryName : libraryNameList.split(",")) {
-      SqlLibrary library =
-          requireNonNull(SqlLibrary.of(libraryName),
-              () -> "library does not exist: " + libraryName);
-      list.add(library);
+    List<String> libList = Arrays.asList(libraryNameList.split(","));
+    if (libList.contains(ALL.abbrev) || libList.contains(ALL.fun)) {
+      // Add all the libraries except ALL, STANDARD, SPATIAL for 'all' and '*'.
+      for (SqlLibrary value : values()) {
+        if (value != ALL && value != STANDARD && value != SPATIAL) {
+          list.add(value);
+        }
+      }
+    } else {
+      for (String libraryName : libraryNameList.split(",")) {

Review Comment:
   Why can't you reuse libList here?



-- 
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


[GitHub] [calcite] julianhyde commented on a diff in pull request #3128: [CALCITE-5606] Add SqlLibrary.ALL and change the library of TANH, COSH, SINH to it

Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #3128:
URL: https://github.com/apache/calcite/pull/3128#discussion_r1147929441


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibrary.java:
##########
@@ -94,11 +97,21 @@ public enum SqlLibrary {
   /** Parses a comma-separated string such as "standard,oracle". */
   public static List<SqlLibrary> parse(String libraryNameList) {
     final ImmutableList.Builder<SqlLibrary> list = ImmutableList.builder();
-    for (String libraryName : libraryNameList.split(",")) {
-      SqlLibrary library =
-          requireNonNull(SqlLibrary.of(libraryName),
-              () -> "library does not exist: " + libraryName);
-      list.add(library);
+    List<String> libList = Arrays.asList(libraryNameList.split(","));

Review Comment:
   Does this do the right thing if someone writes 'all,spatial'? I suspect not, but even if it does, it's not clear.
   
   To simplify, change the method contract so that it may contain duplicates, may contain 'all', and may not be sorted. Then deal with expanding 'all' in the code that calls the method.



-- 
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


[GitHub] [calcite] julianhyde commented on a diff in pull request #3128: [CALCITE-5606] Add SqlLibrary.ALL and change the library of TANH, COSH, SINH to it

Posted by "julianhyde (via GitHub)" <gi...@apache.org>.
julianhyde commented on code in PR #3128:
URL: https://github.com/apache/calcite/pull/3128#discussion_r1147924824


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperatorTableFactory.java:
##########
@@ -70,18 +71,44 @@ private SqlLibraryOperatorTableFactory(Class... classes) {
   /** A cache that returns an operator table for a given library (or set of
    * libraries). */
   @SuppressWarnings("methodref.receiver.bound.invalid")
-  private final LoadingCache<ImmutableSet<SqlLibrary>, SqlOperatorTable> cache =
+  private final LoadingCache<CacheKey, SqlOperatorTable> cache =
       CacheBuilder.newBuilder().build(CacheLoader.from(this::create));
 
+  /**
+   * Cache key that contains library set and a flag indicate whether include operators under

Review Comment:
   Can you explain why cacheKey is required? After expanding 'all', the set of SqlLibary is canonical.



-- 
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


[GitHub] [calcite] zoudan commented on a diff in pull request #3128: [CALCITE-5606] Add SqlLibrary.ALL and change the library of TANH, COSH, SINH to it

Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on code in PR #3128:
URL: https://github.com/apache/calcite/pull/3128#discussion_r1147047628


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibrary.java:
##########
@@ -68,7 +69,9 @@ public enum SqlLibrary {
   POSTGRESQL("p", "postgresql"),
   /** A collection of operators that are in Apache Spark but not in standard
    * SQL. */
-  SPARK("s", "spark");
+  SPARK("s", "spark"),
+  /** A collection of operators that could be used in all libraries except STANDARD and SPATIAL. */
+  ALL("*", "all");

Review Comment:
   I'll change 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


[GitHub] [calcite] zoudan commented on a diff in pull request #3128: [CALCITE-5606] Add SqlLibrary.ALL and change the library of TANH, COSH, SINH to it

Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on code in PR #3128:
URL: https://github.com/apache/calcite/pull/3128#discussion_r1160681642


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibrary.java:
##########
@@ -94,11 +97,21 @@ public enum SqlLibrary {
   /** Parses a comma-separated string such as "standard,oracle". */
   public static List<SqlLibrary> parse(String libraryNameList) {
     final ImmutableList.Builder<SqlLibrary> list = ImmutableList.builder();
-    for (String libraryName : libraryNameList.split(",")) {
-      SqlLibrary library =
-          requireNonNull(SqlLibrary.of(libraryName),
-              () -> "library does not exist: " + libraryName);
-      list.add(library);
+    List<String> libList = Arrays.asList(libraryNameList.split(","));

Review Comment:
   Hi @julianhyde, may I ask if you have time to review this PR, thanks very much



-- 
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


[GitHub] [calcite] zoudan commented on a diff in pull request #3128: [CALCITE-5606] Add SqlLibrary.ALL and change the library of TANH, COSH, SINH to it

Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on code in PR #3128:
URL: https://github.com/apache/calcite/pull/3128#discussion_r1147048639


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibrary.java:
##########
@@ -94,11 +97,21 @@ public enum SqlLibrary {
   /** Parses a comma-separated string such as "standard,oracle". */
   public static List<SqlLibrary> parse(String libraryNameList) {
     final ImmutableList.Builder<SqlLibrary> list = ImmutableList.builder();
-    for (String libraryName : libraryNameList.split(",")) {
-      SqlLibrary library =
-          requireNonNull(SqlLibrary.of(libraryName),
-              () -> "library does not exist: " + libraryName);
-      list.add(library);
+    List<String> libList = Arrays.asList(libraryNameList.split(","));
+    if (libList.contains(ALL.abbrev) || libList.contains(ALL.fun)) {
+      // Add all the libraries except ALL, STANDARD, SPATIAL for 'all' and '*'.
+      for (SqlLibrary value : values()) {
+        if (value != ALL && value != STANDARD && value != SPATIAL) {
+          list.add(value);
+        }
+      }
+    } else {
+      for (String libraryName : libraryNameList.split(",")) {

Review Comment:
   nice catch



-- 
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


[GitHub] [calcite] zoudan commented on a diff in pull request #3128: [CALCITE-5606] Add SqlLibrary.ALL and change the library of TANH, COSH, SINH to it

Posted by "zoudan (via GitHub)" <gi...@apache.org>.
zoudan commented on code in PR #3128:
URL: https://github.com/apache/calcite/pull/3128#discussion_r1148560654


##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperatorTableFactory.java:
##########
@@ -70,18 +71,44 @@ private SqlLibraryOperatorTableFactory(Class... classes) {
   /** A cache that returns an operator table for a given library (or set of
    * libraries). */
   @SuppressWarnings("methodref.receiver.bound.invalid")
-  private final LoadingCache<ImmutableSet<SqlLibrary>, SqlOperatorTable> cache =
+  private final LoadingCache<CacheKey, SqlOperatorTable> cache =
       CacheBuilder.newBuilder().build(CacheLoader.from(this::create));
 
+  /**
+   * Cache key that contains library set and a flag indicate whether include operators under

Review Comment:
   Because I want to handle the special case in `DocumentationTest#testAllFunctionsAreDocumented`, e.g. we do not want to include the operators under 'ALL' when we get operators under 'SPARK'.



-- 
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


[GitHub] [calcite] tanclary commented on pull request #3128: [CALCITE-5606] Add SqlLibrary.ALL and change the library of TANH, COSH, SINH to it

Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on PR #3128:
URL: https://github.com/apache/calcite/pull/3128#issuecomment-1483131625

   @zoudan LGTM! I am not a committer but hopefully someone has time to take a look soon.


-- 
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


[GitHub] [calcite] sonarcloud[bot] commented on pull request #3128: [CALCITE-5606] Add SqlLibrary.ALL and change the library of TANH, COSH, SINH to it

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3128:
URL: https://github.com/apache/calcite/pull/3128#issuecomment-1484117674

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=3128)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3128&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3128&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=3128&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=3128&resolved=false&types=CODE_SMELL)
   
   [![88.9%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/60-16px.png '88.9%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3128&metric=new_coverage&view=list) [88.9% Coverage](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3128&metric=new_coverage&view=list)  
   [![4.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/5-16px.png '4.0%')](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3128&metric=new_duplicated_lines_density&view=list) [4.0% Duplication](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=3128&metric=new_duplicated_lines_density&view=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