[GitHub] [calcite] ffmax closed pull request #1400: make every SqlDialect parse their own data type
ffmax closed pull request #1400: make every SqlDialect parse their own data type URL: https://github.com/apache/calcite/pull/1400 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] danny0405 commented on a change in pull request #1397: [CALCITE-3115] Cannot add JdbcRules which have different JdbcConvention to same VolcanoPlanner's RuleSet.
danny0405 commented on a change in pull request #1397: [CALCITE-3115] Cannot
add JdbcRules which have different JdbcConvention to same VolcanoPlanner's
RuleSet.
URL: https://github.com/apache/calcite/pull/1397#discussion_r316480310
##
File path: core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java
##
@@ -262,6 +263,17 @@ private JdbcRules() {
this(clazz, (Predicate) predicate, in, out, relBuilderFactory,
description);
}
+
+/**
+ * Creates name for child JdbcConverter rules in format:
+ * RuleName(out:CONVENTION_NAME)
+ *
+ * @param out output convention
+ * @return rule name
+ */
+protected static String getRuleName(Class clazz, JdbcConvention out) {
+ return String.format(Locale.ROOT, "%s(out:%s)", clazz.getSimpleName(),
out);
+}
Review comment:
Just one question, can we make more effort to move this method to super
class ConverterRule ? It seems that this is a common pattern, it would be nice
if we can unify the naming rules so there is less chance we encounter the
similar problem.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] LiShuMing opened a new pull request #1401: [CALCITE-3280] Add regexp_replace function (Shuming Li)
LiShuMing opened a new pull request #1401: [CALCITE-3280] Add regexp_replace function (Shuming Li) URL: https://github.com/apache/calcite/pull/1401 Add `regexp_replace` function in Calcite as discussed at [CALCITE-3280](https://issues.apache.org/jira/browse/CALCITE-3280). Add `regexp_replace` unit test in `SqlFunctionsTest.java ` and `SqlOperatorBaseTest.java `. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] jinxing64 edited a comment on issue #1324: [CALCITE-3203] When matching materializations, match Project with child of Aggregate
jinxing64 edited a comment on issue #1324: [CALCITE-3203] When matching
materializations, match Project with child of Aggregate
URL: https://github.com/apache/calcite/pull/1324#issuecomment-523786270
@hsyuan
Thanks for your kind reminder.
Actually I think this case can be covered by
https://github.com/apache/calcite/pull/1384
The matching pattern proposed in this PR is like below:
```
query: Project(projects: [$0, *(2, $1)])
Aggregate(groupSet: {0}, groupSets: [{0}], calls:
[SUM($1)])
Scan(table: [hr, emps])
target: Project(projects: [$0, *(2, $1), *(2, $2)])
Aggregate(groupSet: {0}, groupSets: [{0}], calls:
[SUM($1), COUNT()])
Scan(table: [hr, emps])
```
The matching pattern proposed in https://github.com/apache/calcite/pull/1384
is like below:
```
query: Project(projects: [$0, +($1, 2)])
Project(projects: [$1, $3, $4])
Rel-A
target: Project(projects: [$1, +($3, 2)])
Rel-A
```
I think the second pattern is more general and I added the test case in this
PR to https://github.com/apache/calcite/pull/1384
How do you think ? Which one is preferred ?
If https://github.com/apache/calcite/pull/1384 makes sense, it's great if
you can shepherd and take a review when you have time. Then I will close this
PR.
Thanks ~
Jin
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] jinxing64 edited a comment on issue #1324: [CALCITE-3203] When matching materializations, match Project with child of Aggregate
jinxing64 edited a comment on issue #1324: [CALCITE-3203] When matching
materializations, match Project with child of Aggregate
URL: https://github.com/apache/calcite/pull/1324#issuecomment-523786270
@hsyuan
Thanks for your kind reminder.
Actually I think this case can be covered by
https://github.com/apache/calcite/pull/1384
The matching pattern proposed in this PR is like below:
```
query: Project(projects: [$0, *(2, $1)])
Aggregate(groupSet: {0}, groupSets: [{0}], calls:
[SUM($1)])
Scan(table: [hr, emps])
target: Project(projects: [$0, *(2, $1), *(2, $2)])
Aggregate(groupSet: {0}, groupSets: [{0}], calls:
[SUM($1), COUNT()])
Scan(table: [hr, emps])
```
The matching pattern proposed in https://github.com/apache/calcite/pull/1384
is like below:
```
query: Project(projects: [$0, +($1, 2)])
Project(projects: [$1, $3, $4])
Rel-A
target: Project(projects: [$1, +($3, 2)])
Rel-A
```
I think the second pattern is more general and I added the test case in this
PR to https://github.com/apache/calcite/pull/1384
How do you think ? Which one is preferred ?
If https://github.com/apache/calcite/pull/1384 makes sense, it's great if
you can shepherd and take a review. Then I will close this PR.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] danny0405 commented on a change in pull request #1401: [CALCITE-3280] Add regexp_replace function (Shuming Li)
danny0405 commented on a change in pull request #1401: [CALCITE-3280] Add
regexp_replace function (Shuming Li)
URL: https://github.com/apache/calcite/pull/1401#discussion_r316541081
##
File path:
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
##
@@ -313,6 +313,16 @@ private SqlLibraryOperators() {
OperandTypes.INTEGER,
SqlFunctionCategory.STRING);
+ @LibraryOperator(libraries = {MYSQL, POSTGRESQL, ORACLE})
+ public static final SqlFunction REGEXP_REPLACE =
+ new SqlFunction("REGEXP_REPLACE",
+ SqlKind.OTHER_FUNCTION,
+ ReturnTypes.cascade(ReturnTypes.explicit(SqlTypeName.VARCHAR),
+ SqlTypeTransforms.TO_NULLABLE),
Review comment:
I see that MYSQL and Oracle support another 3 optional arguments for
`REGEXP_REPLACE`[1], are we planning to support this ?
[1]
https://dev.mysql.com/doc/refman/8.0/en/regexp.html#function_regexp-replace
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] ffmax opened a new pull request #1400: make every SqlDialect parse their own data type
ffmax opened a new pull request #1400: make every SqlDialect parse their own data type URL: https://github.com/apache/calcite/pull/1400 Every database might have different type or same type but different type name, therefore making every SqlDialect parse their own data type is suitable way. for example, “select cast(col as int) from table” change to hive sql "select cast(col as integer) from table", but "integer" is not allowed in 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite-avatica-go] F21 merged pull request #47: Add more nil checks in error handling.
F21 merged pull request #47: Add more nil checks in error handling. URL: https://github.com/apache/calcite-avatica-go/pull/47 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[calcite-avatica-go] branch master updated: [CALCITE-3275] add nil checks to error parsing (Tino Rusch).
This is an automated email from the ASF dual-hosted git repository.
francischuang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite-avatica-go.git
The following commit(s) were added to refs/heads/master by this push:
new 7a35be7 [CALCITE-3275] add nil checks to error parsing (Tino Rusch).
7a35be7 is described below
commit 7a35be7713b740eb9bd8e18d2996ca2c279c27f3
Author: Tino Rusch
AuthorDate: Tue Aug 20 13:33:46 2019 +0200
[CALCITE-3275] add nil checks to error parsing (Tino Rusch).
This commit introduces a small helper method to get the server address from
the
message metadata. Before using this helper the driver would crash the whole
application if the server sends an error message without setting up the
metadata
object.
---
connection.go | 7 +--
generic/generic.go | 15 +++
hsqldb/hsqldb.go | 15 +++
message/helper.go | 14 ++
phoenix/phoenix.go | 15 +++
5 files changed, 36 insertions(+), 30 deletions(-)
diff --git a/connection.go b/connection.go
index ce5d1c1..222cc9d 100644
--- a/connection.go
+++ b/connection.go
@@ -216,11 +216,6 @@ func (c *conn) avaticaErrorToResponseErrorOrError(err
error) error {
return
c.adapter.ErrorResponseToResponseError(avaticaErr.message)
}
- serverAddress := "unknown"
- md := avaticaErr.message.GetMetadata()
- if md != nil {
- serverAddress = md.ServerAddress
- }
return errors.ResponseError{
Exceptions: avaticaErr.message.Exceptions,
ErrorMessage: avaticaErr.message.ErrorMessage,
@@ -228,7 +223,7 @@ func (c *conn) avaticaErrorToResponseErrorOrError(err
error) error {
ErrorCode:errors.ErrorCode(avaticaErr.message.ErrorCode),
SqlState: errors.SQLState(avaticaErr.message.SqlState),
Metadata: {
- ServerAddress: serverAddress,
+ ServerAddress:
message.ServerAddressFromMetadata(avaticaErr.message),
},
}
}
diff --git a/generic/generic.go b/generic/generic.go
index 3ca0bd9..a1ed68d 100644
--- a/generic/generic.go
+++ b/generic/generic.go
@@ -109,16 +109,15 @@ func (a Adapter) GetColumnTypeDefinition(col
*message.ColumnMetaData) *internal.
return column
}
-func (a Adapter) ErrorResponseToResponseError(message *message.ErrorResponse)
errors.ResponseError {
-
+func (a Adapter) ErrorResponseToResponseError(err *message.ErrorResponse)
errors.ResponseError {
return errors.ResponseError{
- Exceptions: message.Exceptions,
- ErrorMessage: message.ErrorMessage,
- Severity: int8(message.Severity),
- ErrorCode:errors.ErrorCode(message.ErrorCode),
- SqlState: errors.SQLState(message.SqlState),
+ Exceptions: err.Exceptions,
+ ErrorMessage: err.ErrorMessage,
+ Severity: int8(err.Severity),
+ ErrorCode:errors.ErrorCode(err.ErrorCode),
+ SqlState: errors.SQLState(err.SqlState),
Metadata: {
- ServerAddress: message.GetMetadata().ServerAddress,
+ ServerAddress: message.ServerAddressFromMetadata(err),
},
}
}
diff --git a/hsqldb/hsqldb.go b/hsqldb/hsqldb.go
index 2cb14cd..2ac6f3b 100644
--- a/hsqldb/hsqldb.go
+++ b/hsqldb/hsqldb.go
@@ -109,16 +109,15 @@ func (a Adapter) GetColumnTypeDefinition(col
*message.ColumnMetaData) *internal.
return column
}
-func (a Adapter) ErrorResponseToResponseError(message *message.ErrorResponse)
errors.ResponseError {
-
+func (a Adapter) ErrorResponseToResponseError(err *message.ErrorResponse)
errors.ResponseError {
return errors.ResponseError{
- Exceptions: message.Exceptions,
- ErrorMessage: message.ErrorMessage,
- Severity: int8(message.Severity),
- ErrorCode:errors.ErrorCode(message.ErrorCode),
- SqlState: errors.SQLState(message.SqlState),
+ Exceptions: err.Exceptions,
+ ErrorMessage: err.ErrorMessage,
+ Severity: int8(err.Severity),
+ ErrorCode:errors.ErrorCode(err.ErrorCode),
+ SqlState: errors.SQLState(err.SqlState),
Metadata: {
- ServerAddress: message.GetMetadata().ServerAddress,
+ ServerAddress: message.ServerAddressFromMetadata(err),
},
}
}
diff --git a/message/helper.go b/message/helper.go
new file mode 100644
index 000..a17ab53
--- /dev/null
+++ b/message/helper.go
@@ -0,0 +1,14 @@
+package message
+
+type MetadataProvider interface {
+ GetMetadata() *RpcMetadata
+}
+
+func ServerAddressFromMetadata(message MetadataProvider) string {
[GitHub] [calcite-avatica-go] F21 commented on issue #47: Add more nil checks in error handling.
F21 commented on issue #47: Add more nil checks in error handling. URL: https://github.com/apache/calcite-avatica-go/pull/47#issuecomment-523771232 thanks @trusch ! 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] jinxing64 commented on issue #1324: [CALCITE-3203] When matching materializations, match Project with child of Aggregate
jinxing64 commented on issue #1324: [CALCITE-3203] When matching
materializations, match Project with child of Aggregate
URL: https://github.com/apache/calcite/pull/1324#issuecomment-523786270
@hsyuan
Thanks for your kind reminder.
Actually I think this case can be replaced by
https://github.com/apache/calcite/pull/1384
The matching pattern proposed in this PR is like below:
```
query: Project(projects: [$0, *(2, $1)])
Aggregate(groupSet: {0}, groupSets: [{0}], calls:
[SUM($1)])
Scan(table: [hr, emps])
target: Project(projects: [$0, *(2, $1), *(2, $2)])
Aggregate(groupSet: {0}, groupSets: [{0}], calls:
[SUM($1), COUNT()])
Scan(table: [hr, emps])
```
The matching pattern proposed in https://github.com/apache/calcite/pull/1384
is like below:
```
query: Project(projects: [$0, +($1, 2)])
Project(projects: [$1, $3, $4])
Rel-A
target: Project(projects: [$1, +($3, 2)])
Rel-A
```
I think the second pattern is more general and I added the test case in this
PR to https://github.com/apache/calcite/pull/1384
How do you think ? Which one is preferred ?
If https://github.com/apache/calcite/pull/1384 makes sense, it's great if
you can shepherd and take a review. Then I will close this PR.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] LiShuMing commented on a change in pull request #1401: [CALCITE-3280] Add regexp_replace function (Shuming Li)
LiShuMing commented on a change in pull request #1401: [CALCITE-3280] Add
regexp_replace function (Shuming Li)
URL: https://github.com/apache/calcite/pull/1401#discussion_r316542621
##
File path:
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
##
@@ -313,6 +313,16 @@ private SqlLibraryOperators() {
OperandTypes.INTEGER,
SqlFunctionCategory.STRING);
+ @LibraryOperator(libraries = {MYSQL, POSTGRESQL, ORACLE})
+ public static final SqlFunction REGEXP_REPLACE =
+ new SqlFunction("REGEXP_REPLACE",
+ SqlKind.OTHER_FUNCTION,
+ ReturnTypes.cascade(ReturnTypes.explicit(SqlTypeName.VARCHAR),
+ SqlTypeTransforms.TO_NULLABLE),
Review comment:
Hi @danny0405 , Thanks for your replies. I have reviewed [other
databases'](https://issues.apache.org/jira/browse/CALCITE-3280?focusedCommentId=16913081=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16913081)
usage about `regexp_replace`. Maybe there is a final answer about this
question.
Do you think we should support this? I can work for it If it's needed.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] LiShuMing commented on a change in pull request #1401: [CALCITE-3280] Add regexp_replace function (Shuming Li)
LiShuMing commented on a change in pull request #1401: [CALCITE-3280] Add
regexp_replace function (Shuming Li)
URL: https://github.com/apache/calcite/pull/1401#discussion_r316542621
##
File path:
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
##
@@ -313,6 +313,16 @@ private SqlLibraryOperators() {
OperandTypes.INTEGER,
SqlFunctionCategory.STRING);
+ @LibraryOperator(libraries = {MYSQL, POSTGRESQL, ORACLE})
+ public static final SqlFunction REGEXP_REPLACE =
+ new SqlFunction("REGEXP_REPLACE",
+ SqlKind.OTHER_FUNCTION,
+ ReturnTypes.cascade(ReturnTypes.explicit(SqlTypeName.VARCHAR),
+ SqlTypeTransforms.TO_NULLABLE),
Review comment:
Hi @danny0405 , Thanks for your replies. I have review [other
databases'](https://issues.apache.org/jira/browse/CALCITE-3280?focusedCommentId=16913081=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16913081)
usage about `regexp_replace`. Maybe there is a final answer about this
question.
Do you think we should support this? I can work for it If it's needed.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite-avatica-go] trusch commented on issue #47: Add more nil checks in error handling.
trusch commented on issue #47: Add more nil checks in error handling. URL: https://github.com/apache/calcite-avatica-go/pull/47#issuecomment-523770284 @F21 This is ready to merge now! 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] danny0405 commented on a change in pull request #1401: [CALCITE-3280] Add regexp_replace function (Shuming Li)
danny0405 commented on a change in pull request #1401: [CALCITE-3280] Add
regexp_replace function (Shuming Li)
URL: https://github.com/apache/calcite/pull/1401#discussion_r316544139
##
File path:
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
##
@@ -313,6 +313,16 @@ private SqlLibraryOperators() {
OperandTypes.INTEGER,
SqlFunctionCategory.STRING);
+ @LibraryOperator(libraries = {MYSQL, POSTGRESQL, ORACLE})
+ public static final SqlFunction REGEXP_REPLACE =
+ new SqlFunction("REGEXP_REPLACE",
+ SqlKind.OTHER_FUNCTION,
+ ReturnTypes.cascade(ReturnTypes.explicit(SqlTypeName.VARCHAR),
+ SqlTypeTransforms.TO_NULLABLE),
Review comment:
It would be nice if we support a MYSQL style `REGEXP_REPLACE`.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] ffmax opened a new pull request #1402: make every SqlDialect parse their own data type
ffmax opened a new pull request #1402: make every SqlDialect parse their own data type URL: https://github.com/apache/calcite/pull/1402 Every database might have different type or same type but different type name, therefore making every SqlDialect parse their own data type is a suitable way. for example, “select cast(col as int) from table” change to hive sql "select cast(col as integer) from table", but "integer" is not allowed in 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] danny0405 commented on a change in pull request #1138: [CALCITE-1581] UDTF like in hive
danny0405 commented on a change in pull request #1138: [CALCITE-1581] UDTF like
in hive
URL: https://github.com/apache/calcite/pull/1138#discussion_r316606865
##
File path: core/src/main/codegen/templates/Parser.jj
##
@@ -1579,15 +1579,38 @@ List SelectList() :
SqlNode SelectItem() :
{
SqlNode e;
-final SqlIdentifier id;
+SqlIdentifier id;
+final List ids = new ArrayList();
+final Span s = span();
}
{
e = SelectExpression()
[
[ ]
-id = SimpleIdentifier() {
-e = SqlStdOperatorTable.AS.createCall(span().end(e), e, id);
-}
+(
+(
+ id = SimpleIdentifier()
+ {
+ e = SqlStdOperatorTable.AS.createCall(s.end(e), e, id);
+ }
+)
+|
+(
+
+ id = SimpleIdentifier()
+ {
+ids.add(id);
+ }
+ ( id = SimpleIdentifier() { ids.add(id);} )*
+
+ {
+ if (!this.conformance.allowSelectTableFunction()) {
+throw new
ParseException(RESOURCE.notAllowTableFunctionInSelect().str());
+ }
Review comment:
Thanks, the new change looks fine ~
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] pengzhiwei2018 commented on a change in pull request #1138: [CALCITE-1581] UDTF like in hive
pengzhiwei2018 commented on a change in pull request #1138: [CALCITE-1581] UDTF
like in hive
URL: https://github.com/apache/calcite/pull/1138#discussion_r316597742
##
File path: core/src/main/codegen/templates/Parser.jj
##
@@ -1579,15 +1579,38 @@ List SelectList() :
SqlNode SelectItem() :
{
SqlNode e;
-final SqlIdentifier id;
+SqlIdentifier id;
+final List ids = new ArrayList();
+final Span s = span();
}
{
e = SelectExpression()
[
[ ]
-id = SimpleIdentifier() {
-e = SqlStdOperatorTable.AS.createCall(span().end(e), e, id);
-}
+(
+(
+ id = SimpleIdentifier()
+ {
+ e = SqlStdOperatorTable.AS.createCall(s.end(e), e, id);
+ }
+)
+|
+(
+
+ id = SimpleIdentifier()
+ {
+ids.add(id);
+ }
+ ( id = SimpleIdentifier() { ids.add(id);} )*
+
+ {
+ if (!this.conformance.allowSelectTableFunction()) {
+throw new
ParseException(RESOURCE.notAllowTableFunctionInSelect().str());
+ }
Review comment:
I have pushed the change, but this page has not refresh to the latest code.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] rubenada commented on a change in pull request #1156: [CALCITE-2973] Allow theta joins that have equi conditions to be exec…
rubenada commented on a change in pull request #1156: [CALCITE-2973] Allow
theta joins that have equi conditions to be exec…
URL: https://github.com/apache/calcite/pull/1156#discussion_r316608509
##
File path:
linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java
##
@@ -1054,7 +1054,31 @@ public void close() {
resultSelector,
comparer,
generateNullsOnLeft,
-generateNullsOnRight);
+generateNullsOnRight, null);
Review comment:
minor: to be consistent style-wise, `null` should be in a new line (same
with `predicate` in line 1081).
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] Fokko commented on a change in pull request #1393: [CALCITE-3265] Remove the unused variables
Fokko commented on a change in pull request #1393: [CALCITE-3265] Remove the unused variables URL: https://github.com/apache/calcite/pull/1393#discussion_r316624501 ## File path: core/src/main/java/org/apache/calcite/runtime/Matcher.java ## @@ -63,7 +58,6 @@ private Matcher(Automaton automaton, ImmutableBitSet.builder(); startSetBuilder.set(automaton.startState.id); automaton.epsilonSuccessors(automaton.startState.id, startSetBuilder); -startSet = startSetBuilder.build(); Review comment: Still, we don't do anything meaningful with `startSet`. We could change the line to `startSetBuilder.build();` if we don't want to change behavior. WDYT? https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/util/ImmutableBitSet.java#L958-L968 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] danny0405 commented on a change in pull request #1138: [CALCITE-1581] UDTF like in hive
danny0405 commented on a change in pull request #1138: [CALCITE-1581] UDTF like
in hive
URL: https://github.com/apache/calcite/pull/1138#discussion_r316586419
##
File path: core/src/main/codegen/templates/Parser.jj
##
@@ -1579,15 +1579,38 @@ List SelectList() :
SqlNode SelectItem() :
{
SqlNode e;
-final SqlIdentifier id;
+SqlIdentifier id;
+final List ids = new ArrayList();
+final Span s = span();
}
{
e = SelectExpression()
[
[ ]
-id = SimpleIdentifier() {
-e = SqlStdOperatorTable.AS.createCall(span().end(e), e, id);
-}
+(
+(
+ id = SimpleIdentifier()
+ {
+ e = SqlStdOperatorTable.AS.createCall(s.end(e), e, id);
+ }
+)
+|
+(
+
+ id = SimpleIdentifier()
+ {
+ids.add(id);
+ }
+ ( id = SimpleIdentifier() { ids.add(id);} )*
+
+ {
+ if (!this.conformance.allowSelectTableFunction()) {
+throw new
ParseException(RESOURCE.notAllowTableFunctionInSelect().str());
+ }
Review comment:
Please do not throw ParseException directly, instead use
`SqlUtil.newContextException` to have more detailed error message.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] pengzhiwei2018 commented on a change in pull request #1138: [CALCITE-1581] UDTF like in hive
pengzhiwei2018 commented on a change in pull request #1138: [CALCITE-1581] UDTF
like in hive
URL: https://github.com/apache/calcite/pull/1138#discussion_r316592589
##
File path: core/src/main/codegen/templates/Parser.jj
##
@@ -1579,15 +1579,38 @@ List SelectList() :
SqlNode SelectItem() :
{
SqlNode e;
-final SqlIdentifier id;
+SqlIdentifier id;
+final List ids = new ArrayList();
+final Span s = span();
}
{
e = SelectExpression()
[
[ ]
-id = SimpleIdentifier() {
-e = SqlStdOperatorTable.AS.createCall(span().end(e), e, id);
-}
+(
+(
+ id = SimpleIdentifier()
+ {
+ e = SqlStdOperatorTable.AS.createCall(s.end(e), e, id);
+ }
+)
+|
+(
+
+ id = SimpleIdentifier()
+ {
+ids.add(id);
+ }
+ ( id = SimpleIdentifier() { ids.add(id);} )*
+
+ {
+ if (!this.conformance.allowSelectTableFunction()) {
+throw new
ParseException(RESOURCE.notAllowTableFunctionInSelect().str());
+ }
Review comment:
Thanks for you suggestions @danny0405 .
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] rubenada commented on a change in pull request #1156: [CALCITE-2973] Allow theta joins that have equi conditions to be exec…
rubenada commented on a change in pull request #1156: [CALCITE-2973] Allow
theta joins that have equi conditions to be exec…
URL: https://github.com/apache/calcite/pull/1156#discussion_r316607816
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableJoinRule.java
##
@@ -61,12 +61,29 @@
if (!info.isEqui() && join.getJoinType() != JoinRelType.INNER) {
// EnumerableHashJoin only supports equi-join. We can put a filter on top
// if it is an inner join.
- return EnumerableNestedLoopJoin.create(
- left,
- right,
- join.getCondition(),
- join.getVariablesSet(),
- join.getJoinType());
+ RelNode newRel;
+ //if it has equiKeys ,we can create an EnumerableHashJoin with
Review comment:
Minor detail. Please review style in the comment line: whitespace after '//'
and after comma (not before). Same applies for comment in line 74.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] rubenada edited a comment on issue #1156: [CALCITE-2973] Allow theta joins that have equi conditions to be exec…
rubenada edited a comment on issue #1156: [CALCITE-2973] Allow theta joins that have equi conditions to be exec… URL: https://github.com/apache/calcite/pull/1156#issuecomment-523844012 > > > You can remove this change in this PR: > [36e3109#diff-279a3afafffbb0ef4b411e6c0256a456R60](https://github.com/apache/calcite/commit/36e3109fb524c13ca0a08e3ad585785aa5abc18b#diff-279a3afafffbb0ef4b411e6c0256a456R60) @hsyuan , I'm not sure we can do that, since this change affects just EnumerableHashJoin (which will support now non-equi conditions); but there is still EnumerableMergeJoin (which only works with equi conditions) that remains unchanged. Also, I just noticed that an EnumerableHashJoin of type SEMI/ANTI supports only equi-join. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] rubenada commented on a change in pull request #1156: [CALCITE-2973] Allow theta joins that have equi conditions to be exec…
rubenada commented on a change in pull request #1156: [CALCITE-2973] Allow
theta joins that have equi conditions to be exec…
URL: https://github.com/apache/calcite/pull/1156#discussion_r316642445
##
File path:
core/src/test/java/org/apache/calcite/test/enumerable/EnumerableHashJoinTest.java
##
@@ -0,0 +1,139 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.test.enumerable;
+
+import org.apache.calcite.adapter.java.ReflectiveSchema;
+import org.apache.calcite.config.CalciteConnectionProperty;
+import org.apache.calcite.config.Lex;
+import org.apache.calcite.test.CalciteAssert;
+import org.apache.calcite.test.JdbcTest;
+
+import org.junit.Test;
+
+/**
+ * Unit test for
+ * {@link org.apache.calcite.adapter.enumerable.EnumerableHashJoin}.
+ */
+public class EnumerableHashJoinTest {
Review comment:
Could we also add here some tests for SEMI/ANTI join with/without predicate?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] rubenada commented on a change in pull request #1156: [CALCITE-2973] Allow theta joins that have equi conditions to be exec…
rubenada commented on a change in pull request #1156: [CALCITE-2973] Allow
theta joins that have equi conditions to be exec…
URL: https://github.com/apache/calcite/pull/1156#discussion_r316606659
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableJoinRule.java
##
@@ -61,12 +61,29 @@
if (!info.isEqui() && join.getJoinType() != JoinRelType.INNER) {
Review comment:
Is this `if` condition still relevant? The scenario of a non-equi inner join
implemented as an EnumerableHashJoin + Filter should be supported with just a
(new version of) EnumerableHashJoin, which can take the full condition.
I have the impression that we can remove the `if` condition (and the whole
else block) and leave just the if block: if there are equiKeys, generate
EnumerableHashJoin (which now supports equi and non-equi conditions); otherwise
generate EnumerableNestedLoopJoin.
EDIT: I just realized (if I am not mistaken) that EnumerableHashJoin still
does not support non-equi conditions for SEMI/ANTI join types, in those case I
think we should generate EnumerableNestedLoopJoin too.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] rubenada commented on issue #1156: [CALCITE-2973] Allow theta joins that have equi conditions to be exec…
rubenada commented on issue #1156: [CALCITE-2973] Allow theta joins that have equi conditions to be exec… URL: https://github.com/apache/calcite/pull/1156#issuecomment-523844012 > > > You can remove this change in this PR: > [36e3109#diff-279a3afafffbb0ef4b411e6c0256a456R60](https://github.com/apache/calcite/commit/36e3109fb524c13ca0a08e3ad585785aa5abc18b#diff-279a3afafffbb0ef4b411e6c0256a456R60) @hsyuan , I'm not sure we can do that, since this change affects just EnumerableHashJoin (which will support now non-equi conditions); but there is still EnumerableMergeJoin (which only works with equi conditions) that remains unchanged. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] rubenada commented on a change in pull request #1156: [CALCITE-2973] Allow theta joins that have equi conditions to be exec…
rubenada commented on a change in pull request #1156: [CALCITE-2973] Allow
theta joins that have equi conditions to be exec…
URL: https://github.com/apache/calcite/pull/1156#discussion_r316606659
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableJoinRule.java
##
@@ -61,12 +61,29 @@
if (!info.isEqui() && join.getJoinType() != JoinRelType.INNER) {
Review comment:
Is this `if` condition still relevant? The scenario of a non-equi inner join
implemented as an EnumerableHashJoin + Filter should be supported with just a
(new version of) EnumerableHashJoin, which can take the full condition.
I have the impression that we can remove the `if` condition (and the whole
else block) and leave just the if block: if there are equiKeys, generate
EnumerableHashJoin (which now supports equi and non-equi conditions); otherwise
generate EnumerableNestedLoopJoin.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] rubenada commented on a change in pull request #1156: [CALCITE-2973] Allow theta joins that have equi conditions to be exec…
rubenada commented on a change in pull request #1156: [CALCITE-2973] Allow
theta joins that have equi conditions to be exec…
URL: https://github.com/apache/calcite/pull/1156#discussion_r316606659
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableJoinRule.java
##
@@ -61,12 +61,29 @@
if (!info.isEqui() && join.getJoinType() != JoinRelType.INNER) {
Review comment:
Is this `if` condition still relevant? The scenario of a non-equi inner join
implemented as an EnumerableHashJoin + Filter should be supported with just a
(new version of) EnumerableHashJoin, which can take the full condition.
I have the impression that we can remove the `if` condition (and the whole
else block) and leave just the if block: if there are equiKeys, generate
EnumerableHashJoin (which now supports equi and non-equi conditions); otherwise
generate EnumerableNestedLoopJoin.
EDIT: I just realized (if I am not mistaken) that EnumerableHashJoin still
does not support non-equi conditions for SEMI/ANTI join types [1], in those
case I think we should generate EnumerableNestedLoopJoin too.
[1]
https://github.com/apache/calcite/blob/729446005617059c0ce9fef4068087e4ca9ca139/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableHashJoin.java#L147
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] rubenada commented on a change in pull request #1156: [CALCITE-2973] Allow theta joins that have equi conditions to be exec…
rubenada commented on a change in pull request #1156: [CALCITE-2973] Allow
theta joins that have equi conditions to be exec…
URL: https://github.com/apache/calcite/pull/1156#discussion_r316719387
##
File path:
linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java
##
@@ -1065,20 +1089,30 @@ public void close() {
final Function1 innerKeySelector,
final Function2 resultSelector,
final EqualityComparer comparer, final boolean generateNullsOnLeft,
- final boolean generateNullsOnRight) {
+ final boolean generateNullsOnRight, final Predicate2
predicate) {
return new AbstractEnumerable() {
public Enumerator enumerator() {
+/**
+ * the innerToLookUp will refer the inner , if current join
+ * is a right join, we should figure out the right list first, if
+ * not, then keep the original inner here.
+ */
+final Enumerable innerToLookUp = generateNullsOnLeft
+? Linq4j.asEnumerable(inner.toList())
+: inner;
+
final Lookup innerLookup =
comparer == null
-? inner.toLookup(innerKeySelector)
-: inner.toLookup(innerKeySelector, comparer);
+? innerToLookUp.toLookup(innerKeySelector)
+: innerToLookUp
+.toLookup(innerKeySelector, comparer);
return new Enumerator() {
Enumerator outers = outer.enumerator();
Enumerator inners = Linq4j.emptyEnumerator();
- Set unmatchedKeys =
+ List innersUnmatched =
Review comment:
I left a comment in the Jira ticket regarding this particular change. Since
it is a more general topic, we can discuss there.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] ihuzenko commented on a change in pull request #1397: [CALCITE-3115] Cannot add JdbcRules which have different JdbcConvention to same VolcanoPlanner's RuleSet.
ihuzenko commented on a change in pull request #1397: [CALCITE-3115] Cannot add
JdbcRules which have different JdbcConvention to same VolcanoPlanner's RuleSet.
URL: https://github.com/apache/calcite/pull/1397#discussion_r316735641
##
File path: core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java
##
@@ -262,6 +263,17 @@ private JdbcRules() {
this(clazz, (Predicate) predicate, in, out, relBuilderFactory,
description);
}
+
+/**
+ * Creates name for child JdbcConverter rules in format:
+ * RuleName(out:CONVENTION_NAME)
+ *
+ * @param out output convention
+ * @return rule name
+ */
+protected static String getRuleName(Class clazz, JdbcConvention out) {
+ return String.format(Locale.ROOT, "%s(out:%s)", clazz.getSimpleName(),
out);
+}
Review comment:
This pattern was chosen by me, because it's similar to the one in
```ConverterRule``` constructor:
```
description == null
? "ConverterRule(in:" + in + ",out:" + out + ")"
: description)
```
Only difference in my code is that I didn't use **in** rel trait argument,
since for jdbc rules it's the same everywhere. I can move the method to
```ConverterRule```, but then ignoring ```in``` argument won't be good I think.
So, should I move the method up in the hierarchy with pattern like
```RuleName(in:" + in + ",out:" + out + ")"``` ?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] LaiZhou commented on a change in pull request #1156: [CALCITE-2973] Allow theta joins that have equi conditions to be exec…
LaiZhou commented on a change in pull request #1156: [CALCITE-2973] Allow theta
joins that have equi conditions to be exec…
URL: https://github.com/apache/calcite/pull/1156#discussion_r316661758
##
File path:
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableJoinRule.java
##
@@ -61,12 +61,29 @@
if (!info.isEqui() && join.getJoinType() != JoinRelType.INNER) {
Review comment:
yes, since the SEMI/ANTI join that with empty nonEquiConditions is handled
by EnumerableHashJoin, the `if` condition should handle those case.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] LaiZhou commented on a change in pull request #1156: [CALCITE-2973] Allow theta joins that have equi conditions to be exec…
LaiZhou commented on a change in pull request #1156: [CALCITE-2973] Allow theta
joins that have equi conditions to be exec…
URL: https://github.com/apache/calcite/pull/1156#discussion_r316662337
##
File path:
core/src/test/java/org/apache/calcite/test/enumerable/EnumerableHashJoinTest.java
##
@@ -0,0 +1,139 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.test.enumerable;
+
+import org.apache.calcite.adapter.java.ReflectiveSchema;
+import org.apache.calcite.config.CalciteConnectionProperty;
+import org.apache.calcite.config.Lex;
+import org.apache.calcite.test.CalciteAssert;
+import org.apache.calcite.test.JdbcTest;
+
+import org.junit.Test;
+
+/**
+ * Unit test for
+ * {@link org.apache.calcite.adapter.enumerable.EnumerableHashJoin}.
+ */
+public class EnumerableHashJoinTest {
Review comment:
ok, will do 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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] rubenada commented on a change in pull request #1156: [CALCITE-2973] Allow theta joins that have equi conditions to be exec…
rubenada commented on a change in pull request #1156: [CALCITE-2973] Allow
theta joins that have equi conditions to be exec…
URL: https://github.com/apache/calcite/pull/1156#discussion_r316699416
##
File path:
linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java
##
@@ -1091,38 +1125,44 @@ public boolean moveNext() {
return true;
}
if (!outers.moveNext()) {
-if (unmatchedKeys != null) {
- // We've seen everything else. If we are doing a RIGHT or
FULL
- // join (leftNull = true) there are any keys which right but
- // not the left.
- List list = new ArrayList<>();
- for (TKey key : unmatchedKeys) {
-for (TInner tInner : innerLookup.get(key)) {
- list.add(tInner);
-}
- }
- inners = Linq4j.enumerator(list);
+if (innersUnmatched != null) {
+ inners = Linq4j.enumerator(new ArrayList<>(innersUnmatched));
outers.close();
outers = Linq4j.singletonNullEnumerator();
outers.moveNext();
- unmatchedKeys = null; // don't do the 'leftovers' again
+ innersUnmatched = null; // don't do the 'leftovers' again
continue;
}
return false;
}
final TSource outer = outers.current();
- final Enumerable innerEnumerable;
+ Enumerable innerEnumerable;
if (outer == null) {
innerEnumerable = null;
} else {
final TKey outerKey = outerKeySelector.apply(outer);
if (outerKey == null) {
innerEnumerable = null;
} else {
- if (unmatchedKeys != null) {
-unmatchedKeys.remove(outerKey);
- }
innerEnumerable = innerLookup.get(outerKey);
+ //apply predicate to filter per-row
+ if (innerEnumerable != null && innerEnumerable.any()) {
Review comment:
@LaiZhou I think this new piece of code could be refactored to avoid using
`innerEnumerable.any()` (which internally creates an extra enumerator), and the
local variable `innersToFilter`. I haven't tested the following code, but I
think it should achieve the same result:
```
if (innerEnumerable != null) {
final List matchedInners;
if (predicate == null) {
matchedInners = innerEnumerable.toList();
}
else {
matchedInners = new ArrayList<>();
try (Enumerator innerEnumerator =
innerEnumerable.enumerator()) {
while (innerEnumerator.moveNext()) {
final TInner inner = innerEnumerator.current();
if (predicate.apply(outer, inner)) {
matchedInners.add(inner);
}
}
}
innerEnumerable = Linq4j.asEnumerable(matchedInners);
}
if (innersUnmatched != null) {
innersUnmatched.removeAll(matchedInners);
}
}
```
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] rubenada commented on a change in pull request #1156: [CALCITE-2973] Allow theta joins that have equi conditions to be exec…
rubenada commented on a change in pull request #1156: [CALCITE-2973] Allow
theta joins that have equi conditions to be exec…
URL: https://github.com/apache/calcite/pull/1156#discussion_r316672026
##
File path:
linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java
##
@@ -1063,22 +1084,27 @@ public void close() {
final Enumerable outer, final Enumerable inner,
final Function1 outerKeySelector,
final Function1 innerKeySelector,
+ final Predicate2 predicate,
final Function2 resultSelector,
final EqualityComparer comparer, final boolean generateNullsOnLeft,
final boolean generateNullsOnRight) {
return new AbstractEnumerable() {
public Enumerator enumerator() {
+final Enumerable innerToLookUp = generateNullsOnLeft
Review comment:
@LaiZhou , sorry, long time since I last checked this code. Could you please
remind me why if the join is a RIGHT (or FULL, I would add) we "should figure
out the right list first" ?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] rubenada commented on a change in pull request #1156: [CALCITE-2973] Allow theta joins that have equi conditions to be exec…
rubenada commented on a change in pull request #1156: [CALCITE-2973] Allow
theta joins that have equi conditions to be exec…
URL: https://github.com/apache/calcite/pull/1156#discussion_r316670865
##
File path:
linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java
##
@@ -1091,38 +1125,44 @@ public boolean moveNext() {
return true;
}
if (!outers.moveNext()) {
-if (unmatchedKeys != null) {
- // We've seen everything else. If we are doing a RIGHT or
FULL
- // join (leftNull = true) there are any keys which right but
- // not the left.
- List list = new ArrayList<>();
- for (TKey key : unmatchedKeys) {
-for (TInner tInner : innerLookup.get(key)) {
- list.add(tInner);
-}
- }
- inners = Linq4j.enumerator(list);
+if (innersUnmatched != null) {
+ inners = Linq4j.enumerator(new ArrayList<>(innersUnmatched));
Review comment:
Do we really need a new list here? Couldn't this just be:
`inners = Linq4j.enumerator(innersUnmatched);`
?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] hsyuan commented on issue #1156: [CALCITE-2973] Allow theta joins that have equi conditions to be exec…
hsyuan commented on issue #1156: [CALCITE-2973] Allow theta joins that have equi conditions to be exec… URL: https://github.com/apache/calcite/pull/1156#issuecomment-524000289 > @hsyuan , I'm not sure we can do that, since this change affects just EnumerableHashJoin (which will support now non-equi conditions); but there is still EnumerableMergeJoin (which only works with equi conditions) that remains unchanged. Also, I just noticed that an EnumerableHashJoin of type SEMI/ANTI supports only equi-join. @rubenada You are right, it seems we can't still remove it unless both of them support non-equi conditions. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] julianhyde commented on a change in pull request #1397: [CALCITE-3115] Cannot add JdbcRules which have different JdbcConvention to same VolcanoPlanner's RuleSet.
julianhyde commented on a change in pull request #1397: [CALCITE-3115] Cannot
add JdbcRules which have different JdbcConvention to same VolcanoPlanner's
RuleSet.
URL: https://github.com/apache/calcite/pull/1397#discussion_r316816217
##
File path: core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java
##
@@ -262,6 +263,17 @@ private JdbcRules() {
this(clazz, (Predicate) predicate, in, out, relBuilderFactory,
description);
}
+
+/**
+ * Creates name for child JdbcConverter rules in format:
+ * RuleName(out:CONVENTION_NAME)
+ *
+ * @param out output convention
+ * @return rule name
+ */
+protected static String getRuleName(Class clazz, JdbcConvention out) {
+ return String.format(Locale.ROOT, "%s(out:%s)", clazz.getSimpleName(),
out);
+}
Review comment:
The problem only exists for JdbcConvention, right? Other conventions are
singletons. If so, I would be OK if this code stayed in JdbcRules.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] xndai commented on a change in pull request #1403: [CALCITE-3283] RelSubSet's best is not existed in the set
xndai commented on a change in pull request #1403: [CALCITE-3283] RelSubSet's
best is not existed in the set
URL: https://github.com/apache/calcite/pull/1403#discussion_r316952608
##
File path:
core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
##
@@ -176,6 +176,14 @@
public static final CalciteSystemProperty TEST_SLOW =
booleanProperty("calcite.test.slow", false);
+ /**
+ * Whether to do validation within VolcanoPlanner after each rule firing.
+ * Note that doing so would significantly slow down the planning. Should only
+ * enable for unit test.
+ */
+ public static final CalciteSystemProperty
TEST_VALIDATE_VOLCANO_PLANNER =
+ booleanProperty("calcite.test.validate.volcano.planner", false);
Review comment:
Do you mean log trace level? I update this because I don't want to tight it
with log level. I hope in the future we can turn this check on by default in
unit test. It doesn't necessarily enable tracing at the same 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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] danny0405 commented on a change in pull request #1397: [CALCITE-3115] Cannot add JdbcRules which have different JdbcConvention to same VolcanoPlanner's RuleSet.
danny0405 commented on a change in pull request #1397: [CALCITE-3115] Cannot
add JdbcRules which have different JdbcConvention to same VolcanoPlanner's
RuleSet.
URL: https://github.com/apache/calcite/pull/1397#discussion_r316954972
##
File path: core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java
##
@@ -262,6 +263,17 @@ private JdbcRules() {
this(clazz, (Predicate) predicate, in, out, relBuilderFactory,
description);
}
+
+/**
+ * Creates name for child JdbcConverter rules in format:
+ * RuleName(out:CONVENTION_NAME)
+ *
+ * @param out output convention
+ * @return rule name
+ */
+protected static String getRuleName(Class clazz, JdbcConvention out) {
+ return String.format(Locale.ROOT, "%s(out:%s)", clazz.getSimpleName(),
out);
+}
Review comment:
Even though other conventions are singletons, we didn't use it as part of
the description, i.e. the ElasticSearchConverter rules [1], i believe they have
the same issue.
[1]
https://github.com/apache/calcite/blob/22577e488e04d4056e15af96bac4246fd810f0fa/elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchRules.java#L240
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] danny0405 commented on a change in pull request #1403: [CALCITE-3283] RelSubSet's best is not existed in the set
danny0405 commented on a change in pull request #1403: [CALCITE-3283]
RelSubSet's best is not existed in the set
URL: https://github.com/apache/calcite/pull/1403#discussion_r316956324
##
File path:
core/src/main/java/org/apache/calcite/plan/volcano/VolcanoPlanner.java
##
@@ -1440,6 +1447,11 @@ void rename(RelNode rel) {
boolean existed = subset.set.rels.remove(rel);
assert existed : "rel was not known to its set";
final RelSubset equivSubset = getSubset(equivRel);
+if (subset.best == rel) {
+ subset.best = equivRel;
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] julianhyde commented on a change in pull request #1397: [CALCITE-3115] Cannot add JdbcRules which have different JdbcConvention to same VolcanoPlanner's RuleSet.
julianhyde commented on a change in pull request #1397: [CALCITE-3115] Cannot
add JdbcRules which have different JdbcConvention to same VolcanoPlanner's
RuleSet.
URL: https://github.com/apache/calcite/pull/1397#discussion_r316976211
##
File path: core/src/main/java/org/apache/calcite/adapter/jdbc/JdbcRules.java
##
@@ -262,6 +263,17 @@ private JdbcRules() {
this(clazz, (Predicate) predicate, in, out, relBuilderFactory,
description);
}
+
+/**
+ * Creates name for child JdbcConverter rules in format:
+ * RuleName(out:CONVENTION_NAME)
+ *
+ * @param out output convention
+ * @return rule name
+ */
+protected static String getRuleName(Class clazz, JdbcConvention out) {
+ return String.format(Locale.ROOT, "%s(out:%s)", clazz.getSimpleName(),
out);
+}
Review comment:
Well, exactly. If there is only one instance of convention C, you can only
have one instance of a rule that uses C. But if there are two instances of the
rule that use c1 and c2, you have to include c1 and c2 in the name of those
rules so that their names are different.
Maybe ElasticSearch convention should NOT be a singleton. I suppose we'll
run into the bug when someone wants to run a federated query across more than
one ElasticSearch database.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] julianhyde commented on a change in pull request #1403: [CALCITE-3283] RelSubSet's best is not existed in the set
julianhyde commented on a change in pull request #1403: [CALCITE-3283]
RelSubSet's best is not existed in the set
URL: https://github.com/apache/calcite/pull/1403#discussion_r316977414
##
File path:
core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
##
@@ -176,6 +176,14 @@
public static final CalciteSystemProperty TEST_SLOW =
booleanProperty("calcite.test.slow", false);
+ /**
+ * Whether to do validation within VolcanoPlanner after each rule firing.
+ * Note that doing so would significantly slow down the planning. Should only
+ * enable for unit test.
+ */
+ public static final CalciteSystemProperty
TEST_VALIDATE_VOLCANO_PLANNER =
+ booleanProperty("calcite.test.validate.volcano.planner", false);
Review comment:
I don't want to add too many system properties that are meaningless to
end-users.
You can add a tracer that doesn't print stuff, just controls validation.
It's dangerous to turn extra validation on in tests. The validation can
cause side-effects that cause code to work in tests, fail in production.
Another idea is to do the extra validation if asserts are enabled (`-ea`).
Run the tests with asserts enabled and disabled.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] xndai commented on a change in pull request #1403: [CALCITE-3283] RelSubSet's best is not existed in the set
xndai commented on a change in pull request #1403: [CALCITE-3283] RelSubSet's
best is not existed in the set
URL: https://github.com/apache/calcite/pull/1403#discussion_r316979608
##
File path:
core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
##
@@ -176,6 +176,14 @@
public static final CalciteSystemProperty TEST_SLOW =
booleanProperty("calcite.test.slow", false);
+ /**
+ * Whether to do validation within VolcanoPlanner after each rule firing.
+ * Note that doing so would significantly slow down the planning. Should only
+ * enable for unit test.
+ */
+ public static final CalciteSystemProperty
TEST_VALIDATE_VOLCANO_PLANNER =
+ booleanProperty("calcite.test.validate.volcano.planner", false);
Review comment:
The validation shouldn't have any side-effects. If there are any, I think we
should just fix them. IMO the extra validation is quite useful as we have seen
different consistency issues with the memo.
-ea seems to be a good idea. Will look into that.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] julianhyde commented on a change in pull request #1403: [CALCITE-3283] RelSubSet's best is not existed in the set
julianhyde commented on a change in pull request #1403: [CALCITE-3283]
RelSubSet's best is not existed in the set
URL: https://github.com/apache/calcite/pull/1403#discussion_r316979955
##
File path:
core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
##
@@ -176,6 +176,14 @@
public static final CalciteSystemProperty TEST_SLOW =
booleanProperty("calcite.test.slow", false);
+ /**
+ * Whether to do validation within VolcanoPlanner after each rule firing.
+ * Note that doing so would significantly slow down the planning. Should only
+ * enable for unit test.
+ */
+ public static final CalciteSystemProperty
TEST_VALIDATE_VOLCANO_PLANNER =
+ booleanProperty("calcite.test.validate.volcano.planner", false);
Review comment:
Maybe not in this case, but validation CAN have side effects such as
ensuring that a cache or memo is populated (e.g.
`AbstractRelNode.deriveRowType()`); in production, the cache is not populated,
so the code fails.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[calcite] 02/02: [CALCITE-3220] JDBC adapter now transforms TRIM to TRIM, LTRIM or RTRIM when target is Hive (Jacky Woo)
This is an automated email from the ASF dual-hosted git repository.
jhyde pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 48c0db1206b17c571d477d912d193d32b5be1eff
Author: jackyWoo
AuthorDate: Tue Jul 30 20:15:26 2019 +0800
[CALCITE-3220] JDBC adapter now transforms TRIM to TRIM, LTRIM or RTRIM
when target is Hive (Jacky Woo)
Close apache/calcite#1342
---
.../apache/calcite/sql/dialect/HiveSqlDialect.java | 34 ++--
.../calcite/rel/rel2sql/RelToSqlConverterTest.java | 36 ++
2 files changed, 68 insertions(+), 2 deletions(-)
diff --git
a/core/src/main/java/org/apache/calcite/sql/dialect/HiveSqlDialect.java
b/core/src/main/java/org/apache/calcite/sql/dialect/HiveSqlDialect.java
index 9d72626..2525460 100644
--- a/core/src/main/java/org/apache/calcite/sql/dialect/HiveSqlDialect.java
+++ b/core/src/main/java/org/apache/calcite/sql/dialect/HiveSqlDialect.java
@@ -19,11 +19,13 @@ package org.apache.calcite.sql.dialect;
import org.apache.calcite.config.NullCollation;
import org.apache.calcite.sql.SqlCall;
import org.apache.calcite.sql.SqlDialect;
+import org.apache.calcite.sql.SqlLiteral;
import org.apache.calcite.sql.SqlNode;
import org.apache.calcite.sql.SqlOperator;
import org.apache.calcite.sql.SqlSyntax;
import org.apache.calcite.sql.SqlWriter;
import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.fun.SqlTrimFunction;
/**
* A SqlDialect implementation for the Apache Hive database.
@@ -64,8 +66,8 @@ public class HiveSqlDialect extends SqlDialect {
return null;
}
- @Override public void unparseCall(final SqlWriter writer, final SqlCall
call, final int leftPrec,
- final int rightPrec) {
+ @Override public void unparseCall(final SqlWriter writer, final SqlCall call,
+ final int leftPrec, final int rightPrec) {
switch (call.getKind()) {
case POSITION:
final SqlWriter.Frame frame = writer.startFunCall("INSTR");
@@ -82,11 +84,39 @@ public class HiveSqlDialect extends SqlDialect {
SqlOperator op = SqlStdOperatorTable.PERCENT_REMAINDER;
SqlSyntax.BINARY.unparse(writer, op, call, leftPrec, rightPrec);
break;
+case TRIM:
+ unparseTrim(writer, call, leftPrec, rightPrec);
+ break;
default:
super.unparseCall(writer, call, leftPrec, rightPrec);
}
}
+ /**
+ * For usage of TRIM, LTRIM and RTRIM in Hive, see
+ * https://cwiki.apache.org/confluence/display/Hive/LanguageManual+UDF;>Hive
UDF usage.
+ */
+ private void unparseTrim(SqlWriter writer, SqlCall call, int leftPrec,
+ int rightPrec) {
+assert call.operand(0) instanceof SqlLiteral : call.operand(0);
+SqlLiteral flag = call.operand(0);
+final String operatorName;
+switch (flag.getValueAs(SqlTrimFunction.Flag.class)) {
+case LEADING:
+ operatorName = "LTRIM";
+ break;
+case TRAILING:
+ operatorName = "RTRIM";
+ break;
+default:
+ operatorName = call.getOperator().getName();
+ break;
+}
+final SqlWriter.Frame frame = writer.startFunCall(operatorName);
+call.operand(2).unparse(writer, leftPrec, rightPrec);
+writer.endFunCall(frame);
+ }
+
@Override public boolean supportsCharSet() {
return false;
}
diff --git
a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
index f013e1f..5bebccc 100644
---
a/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
+++
b/core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java
@@ -804,6 +804,42 @@ public class RelToSqlConverterTest {
}
/** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-3220;>[CALCITE-3220]
+ * HiveSqlDialect should transform the SQL-standard TRIM function to TRIM,
+ * LTRIM or RTRIM. */
+ @Test public void testHiveTrim() {
+final String query = "SELECT TRIM(' str ')\n"
++ "from \"foodmart\".\"reserve_employee\"";
+final String expected = "SELECT TRIM(' str ')\n"
++ "FROM foodmart.reserve_employee";
+sql(query).withHive().ok(expected);
+ }
+
+ @Test public void testHiveTrimWithBoth() {
+final String query = "SELECT TRIM(both ' ' from ' str ')\n"
++ "from \"foodmart\".\"reserve_employee\"";
+final String expected = "SELECT TRIM(' str ')\n"
++ "FROM foodmart.reserve_employee";
+sql(query).withHive().ok(expected);
+ }
+
+ @Test public void testHiveTrimWithLeading() {
+final String query = "SELECT TRIM(LEADING ' ' from ' str ')\n"
++ "from \"foodmart\".\"reserve_employee\"";
+final String expected = "SELECT LTRIM(' str ')\n"
++ "FROM foodmart.reserve_employee";
+sql(query).withHive().ok(expected);
+ }
+
+ @Test public void testHiveTrimWithTailing() {
+final String query = "SELECT TRIM(TRAILING ' ' from ' str
[calcite] branch master updated (22577e4 -> 48c0db1)
This is an automated email from the ASF dual-hosted git repository. jhyde pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git. from 22577e4 [CALCITE-3228] Error while applying rule ProjectScanRule: interpreter new 00ad79b [CALCITE-3263] Add MD5, SHA1 SQL functions (Shuming Li) new 48c0db1 [CALCITE-3220] JDBC adapter now transforms TRIM to TRIM, LTRIM or RTRIM when target is Hive (Jacky Woo) The 2 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference. Summary of changes: .../calcite/adapter/enumerable/RexImpTable.java| 4 +++ .../org/apache/calcite/runtime/SqlFunctions.java | 21 + .../apache/calcite/sql/dialect/HiveSqlDialect.java | 34 ++-- .../calcite/sql/fun/SqlLibraryOperators.java | 20 .../org/apache/calcite/util/BuiltInMethod.java | 2 ++ .../calcite/rel/rel2sql/RelToSqlConverterTest.java | 36 ++ .../calcite/sql/test/SqlOperatorBaseTest.java | 34 .../org/apache/calcite/test/SqlFunctionsTest.java | 30 ++ site/_docs/reference.md| 22 +++-- 9 files changed, 191 insertions(+), 12 deletions(-)
[GitHub] [calcite] asfgit closed pull request #1342: [CALCITE-3220] HiveSqlDialect transforms function Trim to correct Hive format
asfgit closed pull request #1342: [CALCITE-3220] HiveSqlDialect transforms function Trim to correct Hive format URL: https://github.com/apache/calcite/pull/1342 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] LiShuMing commented on issue #1390: [CALCITE-3263] Add MD5/SHA1 SQL functions (ShuMing Li)
LiShuMing commented on issue #1390: [CALCITE-3263] Add MD5/SHA1 SQL functions (ShuMing Li) URL: https://github.com/apache/calcite/pull/1390#issuecomment-524180544 I'm sorry. Why close 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] julianhyde commented on a change in pull request #1393: [CALCITE-3265] Remove the unused variables
julianhyde commented on a change in pull request #1393: [CALCITE-3265] Remove the unused variables URL: https://github.com/apache/calcite/pull/1393#discussion_r316824569 ## File path: core/src/main/java/org/apache/calcite/runtime/Matcher.java ## @@ -63,7 +58,6 @@ private Matcher(Automaton automaton, ImmutableBitSet.builder(); startSetBuilder.set(automaton.startState.id); automaton.epsilonSuccessors(automaton.startState.id, startSetBuilder); -startSet = startSetBuilder.build(); Review comment: `Matcher` is a work in progress, because we only have a basic implementation of `MATCH_RECOGNIZE` right now. I don't know what we will eventually need. We might need `startSetBuilder` when we implement the `classifier()` operator in `MATCH_RECOGNIZE`. Let's not try to tidy up that code right now. It's a construction site. There are things in it that are not used now that I added for a reason because I was thinking about the whole problem. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] xndai opened a new pull request #1403: [CALCITE-3283] RelSubSet's best is not existed in the set
xndai opened a new pull request #1403: [CALCITE-3283] RelSubSet's best is not existed in the set URL: https://github.com/apache/calcite/pull/1403 In VolcanoPlanner.rename(), the given relnode will be removed when we find there's an equivalent rel after rename. But if the node to be removed happens to be the best relnode of its subset, the RelSubSet.best will not able to get updated under two scenarios - 1. If equivalent rel is in the same set as the original rel, currently we do nothing. So RelSubSet.best is not updated and points to a node that's removed after rename. 2. If we end up merging two sub set and equivalent rel cost is same as original rel cost, we won't update RelSubSet.best either. Fix the issue by udpdating the RelSubSet.best if best is the RelNode that's supposed to be removed. Also add a new Calcite property TEST_VALIDATE_VOLCANO_PLANNER for turnning on validation during volcano planner rule firing. JIRA - https://issues.apache.org/jira/browse/CALCITE-3283 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] LiShuMing commented on a change in pull request #1401: [CALCITE-3280] Add regexp_replace function (Shuming Li)
LiShuMing commented on a change in pull request #1401: [CALCITE-3280] Add
regexp_replace function (Shuming Li)
URL: https://github.com/apache/calcite/pull/1401#discussion_r316952202
##
File path:
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
##
@@ -313,6 +313,16 @@ private SqlLibraryOperators() {
OperandTypes.INTEGER,
SqlFunctionCategory.STRING);
+ @LibraryOperator(libraries = {MYSQL, POSTGRESQL, ORACLE})
+ public static final SqlFunction REGEXP_REPLACE =
+ new SqlFunction("REGEXP_REPLACE",
+ SqlKind.OTHER_FUNCTION,
+ ReturnTypes.cascade(ReturnTypes.explicit(SqlTypeName.VARCHAR),
+ SqlTypeTransforms.TO_NULLABLE),
Review comment:
OK, I will update it later.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [calcite] hsyuan commented on a change in pull request #1403: [CALCITE-3283] RelSubSet's best is not existed in the set
hsyuan commented on a change in pull request #1403: [CALCITE-3283] RelSubSet's
best is not existed in the set
URL: https://github.com/apache/calcite/pull/1403#discussion_r316905704
##
File path:
core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
##
@@ -176,6 +176,14 @@
public static final CalciteSystemProperty TEST_SLOW =
booleanProperty("calcite.test.slow", false);
+ /**
+ * Whether to do validation within VolcanoPlanner after each rule firing.
+ * Note that doing so would significantly slow down the planning. Should only
+ * enable for unit test.
+ */
+ public static final CalciteSystemProperty
TEST_VALIDATE_VOLCANO_PLANNER =
+ booleanProperty("calcite.test.validate.volcano.planner", false);
Review comment:
I think the flag is not limited to test, so we can remove `test` from the
flag name.
TEST_VALIDATE_VOLCANO_PLANNER -> VALIDATE_VOLCANO_PLANNER
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[calcite] branch master updated (7294460 -> 22577e4)
This is an automated email from the ASF dual-hosted git repository. chunwei pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/calcite.git. from 7294460 [CALCITE-3223] MV fails to match when there is Non-RexInputRef in the projects (Jin Xing) add 22577e4 [CALCITE-3228] Error while applying rule ProjectScanRule: interpreter No new revisions were added by this update. Summary of changes: .../org/apache/calcite/interpreter/Bindables.java | 10 -- .../java/org/apache/calcite/rel/core/Project.java | 13 +--- .../calcite/rel/rules/ProjectTableScanRule.java| 8 ++--- .../org/apache/calcite/test/RelBuilderTest.java| 16 + .../org/apache/calcite/tools/FrameworksTest.java | 38 ++ 5 files changed, 75 insertions(+), 10 deletions(-)
[GitHub] [calcite] chunweilei merged pull request #1355: [CALCITE-3228] Error while applying rule ProjectScanRule: interpreter
chunweilei merged pull request #1355: [CALCITE-3228] Error while applying rule ProjectScanRule: interpreter URL: https://github.com/apache/calcite/pull/1355 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] hbtoo commented on issue #1403: [CALCITE-3283] RelSubSet's best is not existed in the set
hbtoo commented on issue #1403: [CALCITE-3283] RelSubSet's best is not existed in the set URL: https://github.com/apache/calcite/pull/1403#issuecomment-524101474 +1 lgtm 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [calcite] julianhyde commented on a change in pull request #1403: [CALCITE-3283] RelSubSet's best is not existed in the set
julianhyde commented on a change in pull request #1403: [CALCITE-3283]
RelSubSet's best is not existed in the set
URL: https://github.com/apache/calcite/pull/1403#discussion_r316913355
##
File path:
core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
##
@@ -176,6 +176,14 @@
public static final CalciteSystemProperty TEST_SLOW =
booleanProperty("calcite.test.slow", false);
+ /**
+ * Whether to do validation within VolcanoPlanner after each rule firing.
+ * Note that doing so would significantly slow down the planning. Should only
+ * enable for unit test.
+ */
+ public static final CalciteSystemProperty
TEST_VALIDATE_VOLCANO_PLANNER =
+ booleanProperty("calcite.test.validate.volcano.planner", false);
Review comment:
Doesn't seem appropriate as a system property. Maybe a tracing option.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
