Skip to content

Conversation

@Hisoka-X
Copy link
Member

Purpose of this pull request

This PR main do these thing.

  1. Move get shema logic from Config to ReadonlyConfig, CatalogTableUtil will covert Config to ReadonlyConfig, the use it to read schema.
  2. Fix the ReadonlyConfig dosen't keep the order of user config. To achieve it, I rewrite the logic of properties to map to replace jackson.
  3. Add flatten parameter in ConfigUtil as a option to avoid flattening config key. (In schema config, flatten config key makes parsing the schema tricky)
  4. Change CatalogTableUtil::getCatalogTablesFromConfig parameter type from Config to ReadonlyConfig. (In most of time, we invoke it with TableFactoryContext, so we can get ReadonlyConfig directly).

Does this PR introduce any user-facing change?

Yes or no? I rewrite all logic of get schema from config, the expected behavior should remain the same as before, but this is a refactoring of user-facing parts of the code.

How was this patch tested?

add new test.

Check list

@Hisoka-X
Copy link
Member Author

cc @ic4y @ruanwenjun @hailin0

@hailin0
Copy link
Member

hailin0 commented Sep 25, 2023

How to test sort of keys

@Hisoka-X
Copy link
Member Author

How to test sort of keys

In fact the new test treeMapFunctionTest in ConfigUtilTest.java can prove it, but it's not easy to notice, so I modify it, to recheck the order before and after invoke treemap.

Copy link
Member

@ruanwenjun ruanwenjun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@Hisoka-X Hisoka-X merged commit a389f69 into apache:dev Sep 26, 2023
@Hisoka-X Hisoka-X deleted the update-schema-from-config branch September 27, 2023 13:19
gnehil pushed a commit to gnehil/seatunnel that referenced this pull request Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants