[SPARK-49037][SQL][TESTS] Replace schema() with columns()
#47515
Conversation
|
@cloud-fan cc @LuciferYang |
| val table = catalog.loadTable(ident) | ||
| val partTable = new InMemoryPartitionTable( | ||
| table.name(), table.schema(), table.partitioning(), table.properties()) | ||
| table.name(), |
There was a problem hiding this comment.
Would it be better to add a constructor to InMemoryPartitionTable? Also, since InMemoryPartitionTable is code used for testing, it might be acceptable to directly modify the default constructor definition, but I'm not sure if there are third-party projects that depend on this test code.
There was a problem hiding this comment.
I think we can create a new one and let the old one throw an exception?
|
a potential followup of this patch, the deprecated method should have a default implementation or simply be deleted, otherwise each connectors have to implement the deprecated method, which does not make sense. |
I think we can follow the following pattern to get it done. Before that, we need to get support from @cloud-fan , and I will continue. |
|
I'm fine to clean up our own tests, but I'm a little hesitate to make a breaking change. We should discuss it in the dev list first. |
Okay, I will initiate a discussion in the dev list. Thank you very much for your reply. |
|
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable. |
|
@panbingkun Are you still gonna keep working on this PR? It's about to get auto-closed. |
4 participants
What changes were proposed in this pull request?
The pr aims to replace
schema()withcolumns().Why are the changes needed?
StructType schema();(since = "3.4.0") has been marked as@Deprecated. In Spark's internal use, I think we should switch fromschema()tocolumns().Does this PR introduce any user-facing change?
No.
How was this patch tested?
Was this patch authored or co-authored using generative AI tooling?
No.