You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2021/07/02 02:10:05 UTC
[GitHub] [iceberg] haormj opened a new pull request #2778: Core: Fix JdbcCatalog CATALOG_TABLE_NAME to lowercase
haormj opened a new pull request #2778:
URL: https://github.com/apache/iceberg/pull/2778
look https://github.com/apache/iceberg/issues/2744
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] XuQianJin-Stars commented on a change in pull request #2778: Core: Fix JdbcCatalog CATALOG_TABLE_NAME to lowercase
Posted by GitBox <gi...@apache.org>.
XuQianJin-Stars commented on a change in pull request #2778:
URL: https://github.com/apache/iceberg/pull/2778#discussion_r665871338
##########
File path: core/src/main/java/org/apache/iceberg/jdbc/JdbcUtil.java
##########
@@ -27,7 +27,7 @@
import org.apache.iceberg.relocated.com.google.common.collect.Iterables;
final class JdbcUtil {
- protected static final String CATALOG_TABLE_NAME = "ICEBERG_TABLES";
+ protected static final String CATALOG_TABLE_NAME = "iceberg_tables";
Review comment:
> What's the specific reason that we need to change it to lower case ? Just want to align with the other table names ? (I read the context from #2744 but still did not get the point).
PostgreSQL's database kernel is case sensitive, eg: https://www.cnblogs.com/gaofan/p/11398753.html
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] haormj commented on pull request #2778: Core: Fix JdbcCatalog CATALOG_TABLE_NAME to lowercase
Posted by GitBox <gi...@apache.org>.
haormj commented on pull request #2778:
URL: https://github.com/apache/iceberg/pull/2778#issuecomment-872952218
@ismailsimsek of course :-)
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] haormj commented on pull request #2778: Core: Fix JdbcCatalog CATALOG_TABLE_NAME to lowercase
Posted by GitBox <gi...@apache.org>.
haormj commented on pull request #2778:
URL: https://github.com/apache/iceberg/pull/2778#issuecomment-872669254
@ismailsimsek please review :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] ismailsimsek commented on pull request #2778: Core: Fix JdbcCatalog CATALOG_TABLE_NAME to lowercase
Posted by GitBox <gi...@apache.org>.
ismailsimsek commented on pull request #2778:
URL: https://github.com/apache/iceberg/pull/2778#issuecomment-872746858
looks good, do you want o add second fix too ?
changing varchar size like below?
```
"CREATE TABLE " + CATALOG_TABLE_NAME +
"(" +
CATALOG_NAME + " VARCHAR(255) NOT NULL," +
TABLE_NAMESPACE + " VARCHAR(255) NOT NULL," +
TABLE_NAME + " VARCHAR(255) NOT NULL," +
METADATA_LOCATION + " VARCHAR(5500)," +
PREVIOUS_METADATA_LOCATION + " VARCHAR(5500)," +
"PRIMARY KEY (" + CATALOG_NAME + ", " + TABLE_NAMESPACE + ", " + TABLE_NAME + ")" +
")";
```
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on a change in pull request #2778: Core: Fix JdbcCatalog CATALOG_TABLE_NAME to lowercase
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #2778:
URL: https://github.com/apache/iceberg/pull/2778#discussion_r665940364
##########
File path: core/src/main/java/org/apache/iceberg/jdbc/JdbcUtil.java
##########
@@ -27,7 +27,7 @@
import org.apache.iceberg.relocated.com.google.common.collect.Iterables;
final class JdbcUtil {
- protected static final String CATALOG_TABLE_NAME = "ICEBERG_TABLES";
+ protected static final String CATALOG_TABLE_NAME = "iceberg_tables";
Review comment:
OK, got it!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on a change in pull request #2778: Core: Fix JdbcCatalog CATALOG_TABLE_NAME to lowercase
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #2778:
URL: https://github.com/apache/iceberg/pull/2778#discussion_r665855884
##########
File path: core/src/main/java/org/apache/iceberg/jdbc/JdbcUtil.java
##########
@@ -27,7 +27,7 @@
import org.apache.iceberg.relocated.com.google.common.collect.Iterables;
final class JdbcUtil {
- protected static final String CATALOG_TABLE_NAME = "ICEBERG_TABLES";
+ protected static final String CATALOG_TABLE_NAME = "iceberg_tables";
Review comment:
What's the specific reason that we need to change it to lower case ? Just want to align with the other table names ? (I read the context from https://github.com/apache/iceberg/issues/2744 but still did not get the point).
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on a change in pull request #2778: Core: Fix JdbcCatalog CATALOG_TABLE_NAME to lowercase
Posted by GitBox <gi...@apache.org>.
openinx commented on a change in pull request #2778:
URL: https://github.com/apache/iceberg/pull/2778#discussion_r665859255
##########
File path: core/src/main/java/org/apache/iceberg/jdbc/JdbcUtil.java
##########
@@ -42,11 +42,11 @@
protected static final String CREATE_CATALOG_TABLE =
"CREATE TABLE " + CATALOG_TABLE_NAME +
"(" +
- CATALOG_NAME + " VARCHAR(1255) NOT NULL," +
- TABLE_NAMESPACE + " VARCHAR(1255) NOT NULL," +
- TABLE_NAME + " VARCHAR(1255) NOT NULL," +
- METADATA_LOCATION + " VARCHAR(32768)," +
- PREVIOUS_METADATA_LOCATION + " VARCHAR(32768)," +
+ CATALOG_NAME + " VARCHAR(255) NOT NULL," +
+ TABLE_NAMESPACE + " VARCHAR(255) NOT NULL," +
+ TABLE_NAME + " VARCHAR(255) NOT NULL," +
+ METADATA_LOCATION + " VARCHAR(5500)," +
+ PREVIOUS_METADATA_LOCATION + " VARCHAR(5500)," +
"PRIMARY KEY (" + CATALOG_NAME + ", " + TABLE_NAMESPACE + ", " + TABLE_NAME + ")" +
Review comment:
I think the reason to choose 255 for the size of `CATALOG_NAME`, `TABLE_NAMESPACE`, `TABLE_NAME` is : we are composting the three into a primary key, and the primary key size is limited by 767 ( 255 * 3 = 765 < 767).
Okay, that 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx merged pull request #2778: Core: Fix JdbcCatalog CATALOG_TABLE_NAME to lowercase
Posted by GitBox <gi...@apache.org>.
openinx merged pull request #2778:
URL: https://github.com/apache/iceberg/pull/2778
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] openinx commented on pull request #2778: Core: Fix JdbcCatalog CATALOG_TABLE_NAME to lowercase
Posted by GitBox <gi...@apache.org>.
openinx commented on pull request #2778:
URL: https://github.com/apache/iceberg/pull/2778#issuecomment-876202773
Got this merged, thanks @haormj for contributing, and thanks @ismailsimsek @XuQianJin-Stars for reviewing !
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org