You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by "tjbanghart (via GitHub)" <gi...@apache.org> on 2023/10/05 18:46:03 UTC
[PR] [CALCITE-6034] Adds isAutoIncrement and isGenerated arguments to MetaColumn constructor [calcite-avatica]
tjbanghart opened a new pull request, #229:
URL: https://github.com/apache/calcite-avatica/pull/229
(no comment)
--
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-6034] Adds isAutoIncrement and isGenerated arguments to MetaColumn constructor [calcite-avatica]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #229:
URL: https://github.com/apache/calcite-avatica/pull/229#discussion_r1355810616
##########
core/src/main/java/org/apache/calcite/avatica/MetaImpl.java:
##########
@@ -383,11 +405,29 @@ public MetaColumn(
this.charOctetLength = charOctetLength;
this.ordinalPosition = ordinalPosition;
this.isNullable = isNullable;
+ this.isAutoincrement = isAutoincrement;
+ this.isGeneratedcolumn = isGeneratedcolumn;
}
public String getName() {
return columnName;
}
+
+ /** Returns a copy of this MetaColumn, overriding the value of {@code isAutoincrement}. */
+ @SuppressWarnings("unused") // called from Calcite
+ public MetaColumn withIsAutoIncrement(String isAutoincrement) {
Review Comment:
I would make it consistent either do AutoIncremeent or Autoincrement everywhere
--
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-6034] Adds isAutoIncrement and isGenerated arguments to MetaColumn constructor [calcite-avatica]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #229:
URL: https://github.com/apache/calcite-avatica/pull/229#discussion_r1355810095
##########
core/src/main/java/org/apache/calcite/avatica/MetaImpl.java:
##########
@@ -370,6 +371,27 @@ public MetaColumn(
Integer charOctetLength,
int ordinalPosition,
String isNullable) {
+ this(tableCat, tableSchem, tableName, columnName, dataType, typeName, columnSize,
+ decimalDigits, numPrecRadix, nullable, charOctetLength, ordinalPosition, isNullable, "",
+ "");
+ }
+
+ public MetaColumn(
+ String tableCat,
+ String tableSchem,
+ String tableName,
+ String columnName,
+ int dataType,
+ String typeName,
+ Integer columnSize,
+ Integer decimalDigits,
+ Integer numPrecRadix,
+ int nullable,
+ Integer charOctetLength,
+ int ordinalPosition,
+ String isNullable,
+ String isAutoincrement,
Review Comment:
https://stackoverflow.com/questions/44170873/camel-casing-hyphenated-english-words-e-g-re-render#:~:text=1%20Answer&text=In%20camel%2Dcase%2C%20individual%20trailing,a%20single%20word%20would%20normally. this SO user makes a compelling case, sounds good to me
--
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-6034] Adds isAutoIncrement and isGenerated arguments to MetaColumn constructor [calcite-avatica]
Posted by "tjbanghart (via GitHub)" <gi...@apache.org>.
tjbanghart commented on code in PR #229:
URL: https://github.com/apache/calcite-avatica/pull/229#discussion_r1355829878
##########
core/src/main/java/org/apache/calcite/avatica/MetaImpl.java:
##########
@@ -370,6 +371,27 @@ public MetaColumn(
Integer charOctetLength,
int ordinalPosition,
String isNullable) {
+ this(tableCat, tableSchem, tableName, columnName, dataType, typeName, columnSize,
+ decimalDigits, numPrecRadix, nullable, charOctetLength, ordinalPosition, isNullable, "",
+ "");
+ }
+
+ public MetaColumn(
+ String tableCat,
+ String tableSchem,
+ String tableName,
+ String columnName,
+ int dataType,
+ String typeName,
+ Integer columnSize,
+ Integer decimalDigits,
+ Integer numPrecRadix,
+ int nullable,
+ Integer charOctetLength,
+ int ordinalPosition,
+ String isNullable,
+ String isAutoincrement,
Review Comment:
Yeah I guess that's the convention! I see the same in Calcite so probably won't change it here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [CALCITE-6034] Adds isAutoIncrement and isGenerated arguments to MetaColumn constructor [calcite-avatica]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #229:
URL: https://github.com/apache/calcite-avatica/pull/229#discussion_r1355808523
##########
core/src/main/java/org/apache/calcite/avatica/MetaImpl.java:
##########
@@ -352,10 +352,11 @@ public static class MetaColumn implements Named {
public final String scopeTable = null;
public final Short sourceDataType = null;
@ColumnNoNulls
- public final String isAutoincrement = "";
+ public final String isAutoincrement;
@ColumnNoNulls
- public final String isGeneratedcolumn = "";
+ public final String isGeneratedcolumn;
+ @Deprecated // to be removed before 2.0
Review Comment:
I assume this is the constructor that was failing when building Calcite?
--
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-6034] Adds isAutoIncrement and isGenerated arguments to MetaColumn constructor [calcite-avatica]
Posted by "tjbanghart (via GitHub)" <gi...@apache.org>.
tjbanghart commented on code in PR #229:
URL: https://github.com/apache/calcite-avatica/pull/229#discussion_r1355830634
##########
core/src/main/java/org/apache/calcite/avatica/MetaImpl.java:
##########
@@ -383,11 +405,29 @@ public MetaColumn(
this.charOctetLength = charOctetLength;
this.ordinalPosition = ordinalPosition;
this.isNullable = isNullable;
+ this.isAutoincrement = isAutoincrement;
+ this.isGeneratedcolumn = isGeneratedcolumn;
}
public String getName() {
return columnName;
}
+
+ /** Returns a copy of this MetaColumn, overriding the value of {@code isAutoincrement}. */
+ @SuppressWarnings("unused") // called from Calcite
+ public MetaColumn withIsAutoIncrement(String isAutoincrement) {
Review Comment:
lol, I thought about this for too long. I'll change it for consistency.
--
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-6034] Adds isAutoIncrement and isGenerated arguments to MetaColumn constructor [calcite-avatica]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on code in PR #229:
URL: https://github.com/apache/calcite-avatica/pull/229#discussion_r1355809442
##########
core/src/main/java/org/apache/calcite/avatica/MetaImpl.java:
##########
@@ -370,6 +371,27 @@ public MetaColumn(
Integer charOctetLength,
int ordinalPosition,
String isNullable) {
+ this(tableCat, tableSchem, tableName, columnName, dataType, typeName, columnSize,
+ decimalDigits, numPrecRadix, nullable, charOctetLength, ordinalPosition, isNullable, "",
+ "");
+ }
+
+ public MetaColumn(
+ String tableCat,
+ String tableSchem,
+ String tableName,
+ String columnName,
+ int dataType,
+ String typeName,
+ Integer columnSize,
+ Integer decimalDigits,
+ Integer numPrecRadix,
+ int nullable,
+ Integer charOctetLength,
+ int ordinalPosition,
+ String isNullable,
+ String isAutoincrement,
Review Comment:
is auto-increment one word? Do you camelCase hyphenated words? Interesting..
--
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-6034] Adds isAutoIncrement and isGenerated arguments to MetaColumn constructor [calcite-avatica]
Posted by "tanclary (via GitHub)" <gi...@apache.org>.
tanclary commented on PR #229:
URL: https://github.com/apache/calcite-avatica/pull/229#issuecomment-1749784973
nit: update commit message and PR description to be imperative (adds -> add)
--
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-6034] Add isAutoIncrement and isGenerated arguments to MetaColumn constructor [calcite-avatica]
Posted by "tjbanghart (via GitHub)" <gi...@apache.org>.
tjbanghart merged PR #229:
URL: https://github.com/apache/calcite-avatica/pull/229
--
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