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.

Sign up for GitHub

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

Jump to bottom

[Improvement] Add 'catalog-common' module to hold common information needed by query engine #1449

Closed
yuqi1129 opened this issue Jan 11, 2024 · 0 comments · Fixed by #1454
Closed

[Improvement] Add 'catalog-common' module to hold common information needed by query engine #1449

yuqi1129 opened this issue Jan 11, 2024 · 0 comments · Fixed by #1454
Labels
improvement Improvements on everything

Comments

@yuqi1129
Copy link
Contributor

What would you like to be improved?

The query engine needs some property metadata to perform the query, so we should provide this functionality.

How should we improve?

No response
@yuqi1129 yuqi1129 added the improvement Improvements on everything label Jan 11, 2024
This was referenced Jan 11, 2024
Merged
Closed
jerryshao pushed a commit that referenced this issue Jan 15, 2024
@yuqi1129
[#1449] feat(catalogs): Introudce new module bundled-catalog for qu…
04c87b8 
…ery engine. (#1454)

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

- Add a new module `bundled-catalog` to hold common information like
property meta of all catalogs
- Put `bundled-catalog` to `trino-connector` dependency and verify
everything is OK.

### Why are the changes needed?

We need a common module to hold the property meta of all catalogs so
that the query engine depends on it can use it for query.

Fix: #1449 

### Does this PR introduce _any_ user-facing change?

N/A

### How was this patch tested?

Add some UT in module `Trino-connector`
@FANNG1 FANNG1 mentioned this issue Jan 25, 2024
Merged
jerryshao pushed a commit that referenced this issue Jan 26, 2024
@FANNG1
[#1449] fix(jdbc-postgres): audit info is empty (#1716)
81b624a 
### What changes were proposed in this pull request?
to get gravitino identifier id from schema, 
1. get comment from pg
2. add gravitino identifier to properties,
```
//  properties is not changed , we should use the returned properties.
    StringIdentifier.newPropertiesWithId(id, properties);
    JdbcSchema.Builder().withProperties(properties).build();
```

### Why are the changes needed?
we couldn't get audit info from entity store without gravitino
identifier id

Fix: #1449 

### Does this PR introduce _any_ user-facing change?
no

### How was this patch tested?
add test to check audit info
ch3yne pushed a commit to ch3yne/gravitino that referenced this issue Jan 29, 2024
@FANNG1 @ch3yne
[apache#1449] fix(jdbc-postgres): audit info is empty (apache#1716)
a8ea02e 
### What changes were proposed in this pull request?
to get gravitino identifier id from schema, 
1. get comment from pg
2. add gravitino identifier to properties,
```
//  properties is not changed , we should use the returned properties.
    StringIdentifier.newPropertiesWithId(id, properties);
    JdbcSchema.Builder().withProperties(properties).build();
```

### Why are the changes needed?
we couldn't get audit info from entity store without gravitino
identifier id

Fix: apache#1449 

### Does this PR introduce _any_ user-facing change?
no

### How was this patch tested?
add test to check audit info
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
improvement Improvements on everything
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[#1449] feat(catalogs): Introudce new module bundled-catalog for query engine. yuqi1129/gravitino
1 participant
@yuqi1129