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

Case resolver jdbc #2655

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Case resolver jdbc #2655

wants to merge 7 commits into from

Conversation

chngpe
Copy link
Contributor

@chngpe chngpe commented Mar 14, 2025

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Copy link

codecov bot commented Mar 14, 2025

Codecov Report

Attention: Patch coverage is 49.39024% with 166 lines in your changes missing coverage. Please review.

Project coverage is 60.90%. Comparing base (f3521ad) to head (01d42a0).
Report is 147 commits behind head on master.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...ena/connectors/jdbc/resolver/JDBCCaseResolver.java 57.14% 26 Missing and 7 partials ⚠️
...nectors/jdbc/resolver/DefaultJDBCCaseResolver.java 23.07% 30 Missing ⚠️
...athena/connector/lambda/resolver/CaseResolver.java 0.00% 24 Missing ⚠️
.../jdbc/connection/GenericJdbcConnectionFactory.java 0.00% 12 Missing ⚠️
...ena/connectors/redshift/RedshiftRecordHandler.java 0.00% 8 Missing ⚠️
...a/connectors/jdbc/manager/JdbcMetadataHandler.java 69.56% 7 Missing ⚠️
...nnectors/postgresql/PostGreSqlMetadataHandler.java 63.15% 6 Missing and 1 partial ⚠️
...lickhouse/resolver/ClickhouseJDBCCaseResolver.java 42.85% 4 Missing ⚠️
...atalakegen2/resolver/DataLakeGen2CaseResolver.java 42.85% 4 Missing ⚠️
.../athena/connectors/mysql/MySqlMetadataHandler.java 20.00% 3 Missing and 1 partial ⚠️
... and 13 more
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #2655      +/-   ##
============================================
+ Coverage     60.68%   60.90%   +0.22%     
- Complexity     3871     3891      +20     
============================================
  Files           593      602       +9     
  Lines         22130    21928     -202     
  Branches       2732     2702      -30     
============================================
- Hits          13430    13356      -74     
+ Misses         7398     7263     -135     
- Partials       1302     1309       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

chngpe and others added 6 commits March 20, 2025 16:00
… to expand search capabilities only need to provide information schema query

-Clean up duplication code.

-Introduce case resolver to FederationSDK
-Introduce Default JDBC case resolver for JDBC connectors. If we want to expand search capabilities only need to provide information schema query
-Clean up duplication code.
-Fix pagination token, return null when empty or last records.

Standardize Case resolver support for below JDBC connectors:
1. DataLakeGen2
2. Snowflake
3. Oracle
4. Synapse
5. SapHana
6. MySQL
7. PostGreSql
9. ClickHouse

uncomment pom
…per case object with casing_mode = CASE_SENSITIVE_SEARCH
@chngpe chngpe force-pushed the case_resolver_jdbc branch from d9df3ef to ac0155f Compare March 20, 2025 20:01
LOWER, // casing mode to lower case everything (glue and trino lower case everything)
UPPER, // casing mode to upper case everything (oracle by default upper cases everything)
CASE_INSENSITIVE_SEARCH, //
ANNOTATION
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: finish comments here for other two modes

private static final Logger LOGGER = LoggerFactory.getLogger(PostGreSqlJDBCCaseResolver.class);
private static final String SCHEMA_NAME_QUERY_TEMPLATE = "SELECT schema_name FROM information_schema.schemata WHERE lower(schema_name) = ?";
private static final String TABLE_NAME_QUERY_TEMPLATE = "SELECT table_name FROM information_schema.tables WHERE table_schema = ? AND lower(table_name) = ?";
private static final String TABLE_NAME_QUERY_MATERIALIZED_VIEW = "select matviewname as \"table_name\" from pg_catalog.pg_matviews mv where schemaname = ? and lower(matviewname) = ?";
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: SQL keywords are lower case for this but uppercase for other queries

private static final Logger LOGGER = LoggerFactory.getLogger(RedshiftJDBCCaseResolver.class);
private static final String SCHEMA_NAME_QUERY_TEMPLATE = "SELECT nspname FROM pg_namespace WHERE lower(nspname) = ?";
private static final String TABLE_NAME_QUERY_TEMPLATE = "SELECT table_name FROM information_schema.tables WHERE table_schema = ? AND lower(table_name) = ?";
private static final String TABLE_NAME_QUERY_EXTERNAL_TABLE = "SELECT tablename as \"table_name\" FROM svv_external_tables where schemaname = ? and lower(tablename) = ?";
Copy link
Contributor

Choose a reason for hiding this comment

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

lower(tablename) should be lower(table_name) right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants