[#4731] feat(paimon-spark-conenctor): Support delete, update and mergeInto paimon table in paimon spark connector #5864
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 30 commits
December 1, 2024 14:18
…avitino into support-paimon-connector-ddl
…avitino into support-paimon-connector-partition-management
…avitino into support-paimon-connector-ddl
…avitino into support-paimon-connector-ddl
…avitino into support-paimon-connector-partition-management
…b.com:caican00/gravitino into support-paimon-connector-partition-management
…b.com:caican00/gravitino into support-paimon-connector-hive-backend
…ican00/gravitino into support-paimon-connector-jdbc-backend
…ican00/gravitino into support-paimon-connector-function
…00/gravitino into support-paimon-connector-time-travel
…can00/gravitino into support-paimon-connector-extensions
…avitino into support-paimon-connector-extensions
tengqm
reviewed
Dec 16, 2024
| @@ -34,7 +34,7 @@ | |||
| */ | |||
| public class PaimonSchemaPropertiesMetadata extends BasePropertiesMetadata { | |||
|
|
|||
| public static final String COMMENT = "comment"; | |||
| public static final String COMMENT = PaimonConstants.COMMENT; | |||
There was a problem hiding this comment.
Why do we have this constant copied into PaimonSchemaPropertiesMetadata?
Comment on lines
+38
to
+45
| public static final String COMMENT = PaimonConstants.COMMENT; | ||
| public static final String OWNER = PaimonConstants.OWNER; | ||
| public static final String BUCKET_KEY = PaimonConstants.BUCKET_KEY; | ||
| public static final String MERGE_ENGINE = PaimonConstants.MERGE_ENGINE; | ||
| public static final String SEQUENCE_FIELD = PaimonConstants.SEQUENCE_FIELD; | ||
| public static final String ROWKIND_FIELD = PaimonConstants.ROWKIND_FIELD; | ||
| public static final String PRIMARY_KEY = PaimonConstants.PRIMARY_KEY; | ||
| public static final String PARTITION = PaimonConstants.PARTITION; |
There was a problem hiding this comment.
Same as above. Why do we replicate these constants?
| } catch (NoSuchTableException e) { | ||
| throw new RuntimeException( | ||
| String.format( | ||
| "Failed to load the real sparkTable: %s", |
There was a problem hiding this comment.
is sparkeTable a well known term in the community?
|
|
||
| public class PaimonPropertiesConstants { | ||
|
|
||
| public static final String GRAVITINO_PAIMON_CATALOG_BACKEND = PaimonConstants.CATALOG_BACKEND; |
There was a problem hiding this comment.
I'm really confused by this pattern of copying constant definitions.
The referenced constant is defined for Paimon, and we are now inside a Paimon specific class, but we are copying these constants inside the same logical package.
We are giving these constants new names, by prepending many terms.
The result? We get a new public class, giving many existing constants new aliases.
A new Java file, with many long constant names ...
Comment on lines
-30
to
+35
| "hive-3.3", "org.apache.gravitino.spark.connector.hive.GravitinoHiveCatalogSpark33", | ||
| "hive-3.4", "org.apache.gravitino.spark.connector.hive.GravitinoHiveCatalogSpark34", | ||
| "hive-3.5", "org.apache.gravitino.spark.connector.hive.GravitinoHiveCatalogSpark35", | ||
| "hive-3.3", | ||
| "org.apache.gravitino.spark.connector.hive.GravitinoHiveCatalogSpark33", | ||
| "hive-3.4", | ||
| "org.apache.gravitino.spark.connector.hive.GravitinoHiveCatalogSpark34", | ||
| "hive-3.5", | ||
| "org.apache.gravitino.spark.connector.hive.GravitinoHiveCatalogSpark35", |
There was a problem hiding this comment.
These changes really hurts the readability of the source.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Support delete, update and mergeInto paimon table in paimon spark connector
Why are the changes needed?
Fix: #4731
Does this PR introduce any user-facing change?
No.
How was this patch tested?
New ITs.