Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Subtask] support UpdateColumnNullable in AlterTable for spark connector #2649

Closed
FANNG1 opened this issue Mar 22, 2024 · 2 comments · Fixed by #2699
Closed

[Subtask] support UpdateColumnNullable in AlterTable for spark connector #2649

FANNG1 opened this issue Mar 22, 2024 · 2 comments · Fixed by #2699
Assignees
Labels
good first issue Good for newcomers subtask Subtasks of umbrella issue

Comments

@FANNG1
Copy link
Contributor

FANNG1 commented Mar 22, 2024

Describe the subtask

please add related code to com.datastrato.gravitino.spark.catalogGravitinoCatalog.java, transfrom Spark TableChange to Gravitino TableChange, you could refer to #2502 , Hive doesn't support update column nullable for now, so no need to add integrate test.
public Table alterTable(Identifier ident, TableChange... changes) throws NoSuchTableException { }

Parent issue

#1227

@FANNG1 FANNG1 added the subtask Subtasks of umbrella issue label Mar 22, 2024
@FANNG1 FANNG1 added this to the Gravitino 0.5.0 milestone Mar 22, 2024
@FANNG1 FANNG1 added the good first issue Good for newcomers label Mar 22, 2024
@ichuniq
Copy link
Contributor

ichuniq commented Mar 22, 2024

Hi I would like to work on this! However, could you elaborate on

Hive doesn't support update column nullable for now, so no need to add integrate test.

What I understand now is that I probably should add testAlterTableUpdateColumnNullability() in SparkCommonIT.java like the referenced PR did, alongside with converting spark TableChange.UpdateColumnNullavility() and adding its unit test. Did I miss anything? Thanks!

@FANNG1
Copy link
Contributor Author

FANNG1 commented Mar 23, 2024

@ichuniq , you needn't add testAlterTableUpdateColumnNullability() in SparkCommonIT.java, because it wouldn't work. you only need to implement convert logic and add unit test.

FANNG1 pushed a commit that referenced this issue Mar 28, 2024
…erTable for spark connector (#2699)

### What changes were proposed in this pull request?

Add support for `UpdateColumnNullability` when transforming spark's
TableChange to Gravatino's TableChange for spark connector

### Why are the changes needed?

Fix: #2649 

### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
New unit test & run
`./gradlew :spark-connector:test`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers subtask Subtasks of umbrella issue
Projects
None yet
2 participants