You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "zhuyufeng0809 (via GitHub)" <gi...@apache.org> on 2025/10/30 04:21:09 UTC
[PR] [CALCITE-7256] Add getters for SqlTableRef [calcite]
zhuyufeng0809 opened a new pull request, #4609:
URL: https://github.com/apache/calcite/pull/4609
Refer to：[CALCITE-7256](https://issues.apache.org/jira/browse/CALCITE-7256)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7256] Make the fields of SqlTableRef public [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on code in PR #4609:
URL: https://github.com/apache/calcite/pull/4609#discussion_r2496653405
##########
core/src/main/java/org/apache/calcite/sql/SqlTableRef.java:
##########
@@ -36,8 +36,15 @@ public class SqlTableRef extends SqlCall {
//~ Instance fields --------------------------------------------------------
- private final SqlIdentifier tableName;
- private final SqlNodeList hints;
+ /**
+ * Table name identifier that contains the fully qualified name of the table.
Review Comment:
I don't quite understand the logic here. I have a small question: Is "the fully qualified name" correct?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7256] Make the fields of SqlTableRef public [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4609:
URL: https://github.com/apache/calcite/pull/4609#issuecomment-3508071586
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4609) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4609&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4609&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4609&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4609&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4609&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4609)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7256] Add getters for SqlTableRef [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4609:
URL: https://github.com/apache/calcite/pull/4609#issuecomment-3466118440
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4609) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4609&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4609&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4609&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4609&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4609&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4609)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7256] Make the fields of SqlTableRef public [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on code in PR #4609:
URL: https://github.com/apache/calcite/pull/4609#discussion_r2496653405
##########
core/src/main/java/org/apache/calcite/sql/SqlTableRef.java:
##########
@@ -36,8 +36,15 @@ public class SqlTableRef extends SqlCall {
//~ Instance fields --------------------------------------------------------
- private final SqlIdentifier tableName;
- private final SqlNodeList hints;
+ /**
+ * Table name identifier that contains the fully qualified name of the table.
Review Comment:
I have a small question: Is "the fully qualified name" correct?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7256] Make the fields of SqlTableRef public [calcite]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #4609:
URL: https://github.com/apache/calcite/pull/4609#issuecomment-3477510141
## [](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4609) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4609&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite&pullRequest=4609&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite&pullRequest=4609&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4609&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite&pullRequest=4609&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_calcite&pullRequest=4609)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7256] Make the fields of SqlTableRef public [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4609:
URL: https://github.com/apache/calcite/pull/4609#discussion_r2499904813
##########
core/src/main/java/org/apache/calcite/sql/SqlTableRef.java:
##########
@@ -36,8 +36,15 @@ public class SqlTableRef extends SqlCall {
//~ Instance fields --------------------------------------------------------
- private final SqlIdentifier tableName;
- private final SqlNodeList hints;
+ /**
+ * Table name identifier that contains the fully qualified name of the table.
Review Comment:
I would like a test that confirms that this field indeed holds a fully-qualified name, or otherwise the comment should be revised.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7256] Add getters for SqlTableRef [calcite]
Posted by "zhuyufeng0809 (via GitHub)" <gi...@apache.org>.
zhuyufeng0809 commented on PR #4609:
URL: https://github.com/apache/calcite/pull/4609#issuecomment-3466075890
Hi @NobiGo , could you help me to review 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.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7256] Add getters for SqlTableRef [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #4609:
URL: https://github.com/apache/calcite/pull/4609#issuecomment-3469074544
> @zhuyufeng0809 thanks for your contribution, please check the associated Jira ticket as there is some feedback there on how to improve the PR.
>
> @caicancai: let's make sure the feedback gets applied before merging, thanks!
Thanks for reminding me. I haven't reviewed PRs in a long time and overlooked Jira's feedback. I'm very sorry.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7256] Make the fields of SqlTableRef public [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4609:
URL: https://github.com/apache/calcite/pull/4609#discussion_r2496658088
##########
core/src/main/java/org/apache/calcite/sql/SqlTableRef.java:
##########
@@ -36,8 +36,15 @@ public class SqlTableRef extends SqlCall {
//~ Instance fields --------------------------------------------------------
- private final SqlIdentifier tableName;
- private final SqlNodeList hints;
+ /**
+ * Table name identifier that contains the fully qualified name of the table.
Review Comment:
in the code both "fully qualified" and "fully-qualified" appear a lot
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7256] Make the fields of SqlTableRef public [calcite]
Posted by "zhuyufeng0809 (via GitHub)" <gi...@apache.org>.
zhuyufeng0809 commented on code in PR #4609:
URL: https://github.com/apache/calcite/pull/4609#discussion_r2507922455
##########
core/src/main/java/org/apache/calcite/sql/SqlTableRef.java:
##########
@@ -36,8 +36,15 @@ public class SqlTableRef extends SqlCall {
//~ Instance fields --------------------------------------------------------
- private final SqlIdentifier tableName;
- private final SqlNodeList hints;
+ /**
+ * Table name identifier that contains the fully qualified name of the table.
Review Comment:
@mihaibudiu @xiedeyantu Sorry for the late reply. As you mentioned, the grammar file does not mandate fully qualified table names, and I have made the changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7256] Make the fields of SqlTableRef public [calcite]
Posted by "mihaibudiu (via GitHub)" <gi...@apache.org>.
mihaibudiu commented on code in PR #4609:
URL: https://github.com/apache/calcite/pull/4609#discussion_r2496657232
##########
core/src/main/java/org/apache/calcite/sql/SqlTableRef.java:
##########
@@ -36,8 +36,15 @@ public class SqlTableRef extends SqlCall {
//~ Instance fields --------------------------------------------------------
- private final SqlIdentifier tableName;
- private final SqlNodeList hints;
+ /**
+ * Table name identifier that contains the fully qualified name of the table.
Review Comment:
grammatically it is; it is correct with respect to the code only if the identifier always has a full path.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7256] Make the fields of SqlTableRef public [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #4609:
URL: https://github.com/apache/calcite/pull/4609#issuecomment-3484081494
@zhuyufeng0809 This is just one question: Does Calcite have any tests to show whether this change is feasible?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7256] Make the fields of SqlTableRef public [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai merged PR #4609:
URL: https://github.com/apache/calcite/pull/4609
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7256] Make the fields of SqlTableRef public [calcite]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #4609:
URL: https://github.com/apache/calcite/pull/4609#issuecomment-3490473445
@asolimando Do you have any other opinions?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-7256] Make the fields of SqlTableRef public [calcite]
Posted by "xiedeyantu (via GitHub)" <gi...@apache.org>.
xiedeyantu commented on code in PR #4609:
URL: https://github.com/apache/calcite/pull/4609#discussion_r2496676572
##########
core/src/main/java/org/apache/calcite/sql/SqlTableRef.java:
##########
@@ -36,8 +36,15 @@ public class SqlTableRef extends SqlCall {
//~ Instance fields --------------------------------------------------------
- private final SqlIdentifier tableName;
- private final SqlNodeList hints;
+ /**
+ * Table name identifier that contains the fully qualified name of the table.
Review Comment:
Because I noticed that this is just a simple class which might be used in various places. Can we be sure it refers to the fully qualified name here? As you mentioned earlier, an identifier needs to include the full path to be considered "fully qualified." I'm uncertain if this description might be misleading.
--
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