Saphana glue get table api issue fix #2505
Conversation
AbdulR3hman
force-pushed
the
saphana_phase2_tablelist_issue
branch
from
03c1d41 to
f57df7c
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2505 +/- ##
============================================
+ Coverage 61.09% 61.18% +0.09%
- Complexity 3738 3751 +13
============================================
Files 576 577 +1
Lines 21348 21430 +82
Branches 2654 2659 +5
============================================
+ Hits 13043 13113 +70
- Misses 7037 7046 +9
- Partials 1268 1271 +3 ☔ View full report in Codecov by Sentry. |
| @@ -32,15 +32,15 @@ public final class SaphanaConstants | |||
| public static final int SAPHANA_DEFAULT_PORT = 1025; | |||
| static final Map<String, String> JDBC_PROPERTIES = ImmutableMap.of("databaseTerm", "SCHEMA"); | |||
| static final String ALL_PARTITIONS = "0"; | |||
| static final String BLOCK_PARTITION_COLUMN_NAME = "PART_ID"; | |||
| static final String BLOCK_PARTITION_COLUMN_NAME = "PART_ID".toLowerCase(); | |||
There was a problem hiding this comment.
nit; BLOCK_PARTITION_COLUMN_NAME = "part_id";
There was a problem hiding this comment.
updated with above changes.
| GetTableLayoutRequest getTableLayoutRequest = new GetTableLayoutRequest(this.federatedIdentity, "testQueryId", "testCatalogName", tableName, constraints, partitionSchema, partitionCols); | ||
|
|
||
| PreparedStatement preparedStatement = Mockito.mock(PreparedStatement.class); | ||
|
|
||
| Mockito.when(this.connection.prepareStatement(SaphanaConstants.GET_PARTITIONS_QUERY)).thenReturn(preparedStatement); | ||
|
|
||
| String[] columns = {"PART_ID"}; | ||
| String[] columns = {"part_id"}; |
There was a problem hiding this comment.
why not use BLOCK_PARTITION_COLUMN_NAME; also please statically important since it is used in multiple places
| @@ -149,7 +149,7 @@ public void doGetTableLayoutWithNoPartitions() | |||
| PreparedStatement preparedStatement = Mockito.mock(PreparedStatement.class); | |||
| Mockito.when(this.connection.prepareStatement(SaphanaConstants.GET_PARTITIONS_QUERY)).thenReturn(preparedStatement); | |||
|
|
|||
| String[] columns = {"PART_ID"}; | |||
| String[] columns = {"part_id"}; | |||
There was a problem hiding this comment.
same here use the constant!
| @@ -245,7 +245,7 @@ public void doGetSplitsContinuation() | |||
| PreparedStatement preparedStatement = Mockito.mock(PreparedStatement.class); | |||
| Mockito.when(this.connection.prepareStatement(SaphanaConstants.GET_PARTITIONS_QUERY)).thenReturn(preparedStatement); | |||
|
|
|||
| String[] columns = {"PART_ID"}; | |||
| String[] columns = {"part_id"}; | |||
| @@ -260,7 +260,7 @@ public void doGetSplitsContinuation() | |||
| GetSplitsResponse getSplitsResponse = this.saphanaMetadataHandler.doGetSplits(splitBlockAllocator, getSplitsRequest); | |||
|
|
|||
| Set<Map<String, String>> expectedSplits = new HashSet<>(); | |||
| expectedSplits.add(Collections.singletonMap("PART_ID", "p1")); | |||
| expectedSplits.add(Collections.singletonMap("part_id", "p1")); | |||
|
rebasing seems to cause an issue; please investigate https://github.com/awslabs/aws-athena-query-federation/actions/runs/12676924733/job/35331139283?pr=2505 Also please provide testing for both glue and non glue setup. |
I have updated PR branch with latest master. Please find attached results for both glue and non glue test. |
AbdulR3hman
force-pushed
the
saphana_phase2_tablelist_issue
branch
from
50aa269 to
401275f
Compare
Issue #, if available:
Description of changes:
The Saphana Glue Get Table API was not retrieving data as expected due to a case-sensitivity issue with the PART_ID column. By converting the PART_ID column to lowercase, we achieved successful data retrieval through the Glue API.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.