You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "zhuangchong (via GitHub)" <gi...@apache.org> on 2023/04/28 07:33:10 UTC
[GitHub] [incubator-paimon] zhuangchong opened a new pull request, #1051: [hotfix] add column 'already exists' and 'does not exist' exception type
zhuangchong opened a new pull request, #1051:
URL: https://github.com/apache/incubator-paimon/pull/1051
*(Please specify the module before the PR name: [core] ... or [flink] ...)*
### Purpose
*(What is the purpose of the change, or the associated issue)*
this pr closes: #900
### Tests
*(List UT and IT cases to verify this change)*
### API and Format
*(Does this change affect API or storage format)*
### Documentation
*(Does this change introduce a new feature)*
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-paimon] JingsongLi commented on a diff in pull request #1051: [hotfix] add column 'already exists' and 'does not exist' exception type
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #1051:
URL: https://github.com/apache/incubator-paimon/pull/1051#discussion_r1186640148
##########
paimon-core/src/main/java/org/apache/paimon/catalog/Identifier.java:
##########
@@ -37,7 +35,6 @@
public class Identifier implements Serializable {
private static final long serialVersionUID = 1L;
-
Review Comment:
revert this?
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-paimon] zhuangchong closed pull request #1051: [hotfix] add column 'already exists' and 'does not exist' exception type
Posted by "zhuangchong (via GitHub)" <gi...@apache.org>.
zhuangchong closed pull request #1051: [hotfix] add column 'already exists' and 'does not exist' exception type
URL: https://github.com/apache/incubator-paimon/pull/1051
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-paimon] JingsongLi commented on a diff in pull request #1051: [hotfix] add column 'already exists' and 'does not exist' exception type
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #1051:
URL: https://github.com/apache/incubator-paimon/pull/1051#discussion_r1185682566
##########
paimon-core/src/main/java/org/apache/paimon/schema/SchemaManager.java:
##########
@@ -68,12 +70,14 @@ public class SchemaManager implements Serializable {
private final FileIO fileIO;
private final Path tableRoot;
+ private final Identifier identifier;
@Nullable private transient Lock lock;
public SchemaManager(FileIO fileIO, Path tableRoot) {
this.fileIO = fileIO;
this.tableRoot = tableRoot;
+ this.identifier = Identifier.fromPath(tableRoot, true);
Review Comment:
Can we just create a util method?
`Identifier tableIdentifier`
##########
paimon-core/src/main/java/org/apache/paimon/catalog/Identifier.java:
##########
@@ -87,21 +88,41 @@ public static Identifier fromString(String fullName) {
}
public static Identifier fromPath(Path tablePath) {
- return fromPath(tablePath.getPath());
+ return fromPath(tablePath, false);
}
public static Identifier fromPath(String tablePath) {
+ return fromPath(tablePath, false);
+ }
+
+ public static Identifier fromPath(Path tablePath, boolean ignoreIfUnknownDatabase) {
+ return fromPath(tablePath.getPath(), ignoreIfUnknownDatabase);
+ }
+
+ public static Identifier fromPath(String tablePath, boolean ignoreIfUnknownDatabase) {
Review Comment:
Consider `Identifier` is a public api, can we just put these logical in [SchemaManager.java](https://github.com/apache/incubator-paimon/pull/1051/files#diff-fdc47daefd9d3a55f45fde8385a8be476f6834c30649f7f5bae98cbe90f15dbb)?
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-paimon] zhuangchong commented on a diff in pull request #1051: [hotfix] add column 'already exists' and 'does not exist' exception type
Posted by "zhuangchong (via GitHub)" <gi...@apache.org>.
zhuangchong commented on code in PR #1051:
URL: https://github.com/apache/incubator-paimon/pull/1051#discussion_r1185681371
##########
paimon-core/src/main/java/org/apache/paimon/catalog/Catalog.java:
##########
@@ -273,7 +274,15 @@ class TableNotExistException extends Exception {
private static final String MSG = "Table %s does not exist.";
- private final Identifier identifier;
+ private Identifier identifier;
+
+ public TableNotExistException(Path tablePath) {
Review Comment:
done.
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-paimon] JingsongLi commented on a diff in pull request #1051: [hotfix] add column 'already exists' and 'does not exist' exception type
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #1051:
URL: https://github.com/apache/incubator-paimon/pull/1051#discussion_r1184581577
##########
paimon-core/src/main/java/org/apache/paimon/catalog/Catalog.java:
##########
@@ -273,7 +274,15 @@ class TableNotExistException extends Exception {
private static final String MSG = "Table %s does not exist.";
- private final Identifier identifier;
+ private Identifier identifier;
+
+ public TableNotExistException(Path tablePath) {
Review Comment:
Can we just introduce an unknown database for this?
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-paimon] zhuangchong closed pull request #1051: [hotfix] add column 'already exists' and 'does not exist' exception type
Posted by "zhuangchong (via GitHub)" <gi...@apache.org>.
zhuangchong closed pull request #1051: [hotfix] add column 'already exists' and 'does not exist' exception type
URL: https://github.com/apache/incubator-paimon/pull/1051
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-paimon] JingsongLi merged pull request #1051: [hotfix] add column 'already exists' and 'does not exist' exception type
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #1051:
URL: https://github.com/apache/incubator-paimon/pull/1051
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-paimon] zhuangchong commented on a diff in pull request #1051: [hotfix] add column 'already exists' and 'does not exist' exception type
Posted by "zhuangchong (via GitHub)" <gi...@apache.org>.
zhuangchong commented on code in PR #1051:
URL: https://github.com/apache/incubator-paimon/pull/1051#discussion_r1191922836
##########
paimon-core/src/main/java/org/apache/paimon/schema/SchemaManager.java:
##########
@@ -58,22 +60,26 @@
import java.util.function.Function;
import java.util.stream.Collectors;
+import static org.apache.paimon.catalog.Catalog.DB_SUFFIX;
import static org.apache.paimon.utils.FileUtils.listVersionedFiles;
import static org.apache.paimon.utils.Preconditions.checkState;
/** Schema Manager to manage schema versions. */
public class SchemaManager implements Serializable {
private static final String SCHEMA_PREFIX = "schema-";
+ private static final String UNKNOWN_DATABASE = "unknown";
private final FileIO fileIO;
private final Path tableRoot;
+ private final Identifier tableIdentifier;
Review Comment:
done.
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-paimon] zhuangchong commented on a diff in pull request #1051: [hotfix] add column 'already exists' and 'does not exist' exception type
Posted by "zhuangchong (via GitHub)" <gi...@apache.org>.
zhuangchong commented on code in PR #1051:
URL: https://github.com/apache/incubator-paimon/pull/1051#discussion_r1185872776
##########
paimon-core/src/main/java/org/apache/paimon/schema/SchemaManager.java:
##########
@@ -68,12 +70,14 @@ public class SchemaManager implements Serializable {
private final FileIO fileIO;
private final Path tableRoot;
+ private final Identifier identifier;
@Nullable private transient Lock lock;
public SchemaManager(FileIO fileIO, Path tableRoot) {
this.fileIO = fileIO;
this.tableRoot = tableRoot;
+ this.identifier = Identifier.fromPath(tableRoot, true);
Review Comment:
done.
##########
paimon-core/src/main/java/org/apache/paimon/catalog/Identifier.java:
##########
@@ -87,21 +88,41 @@ public static Identifier fromString(String fullName) {
}
public static Identifier fromPath(Path tablePath) {
- return fromPath(tablePath.getPath());
+ return fromPath(tablePath, false);
}
public static Identifier fromPath(String tablePath) {
+ return fromPath(tablePath, false);
+ }
+
+ public static Identifier fromPath(Path tablePath, boolean ignoreIfUnknownDatabase) {
+ return fromPath(tablePath.getPath(), ignoreIfUnknownDatabase);
+ }
+
+ public static Identifier fromPath(String tablePath, boolean ignoreIfUnknownDatabase) {
Review Comment:
done.
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-paimon] JingsongLi commented on a diff in pull request #1051: [hotfix] add column 'already exists' and 'does not exist' exception type
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #1051:
URL: https://github.com/apache/incubator-paimon/pull/1051#discussion_r1186640114
##########
paimon-core/src/main/java/org/apache/paimon/catalog/Catalog.java:
##########
@@ -40,6 +40,8 @@ public interface Catalog extends AutoCloseable {
String SYSTEM_TABLE_SPLITTER = "$";
+ String DB_SUFFIX = ".db";
Review Comment:
revert this?
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-paimon] JingsongLi commented on a diff in pull request #1051: [hotfix] add column 'already exists' and 'does not exist' exception type
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #1051:
URL: https://github.com/apache/incubator-paimon/pull/1051#discussion_r1186640389
##########
paimon-core/src/main/java/org/apache/paimon/schema/SchemaManager.java:
##########
@@ -58,22 +60,26 @@
import java.util.function.Function;
import java.util.stream.Collectors;
+import static org.apache.paimon.catalog.Catalog.DB_SUFFIX;
import static org.apache.paimon.utils.FileUtils.listVersionedFiles;
import static org.apache.paimon.utils.Preconditions.checkState;
/** Schema Manager to manage schema versions. */
public class SchemaManager implements Serializable {
private static final String SCHEMA_PREFIX = "schema-";
+ private static final String UNKNOWN_DATABASE = "unknown";
private final FileIO fileIO;
private final Path tableRoot;
+ private final Identifier tableIdentifier;
Review Comment:
Can this be a local field? Because this unknown identifier is not correct, we can hide it in exception.
--
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@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org