Skip to content

fix: struct metadata parsing for nested decimal fields #982

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

Merged
jayantsing-db merged 5 commits into from
Sep 9, 2025
Merged

fix: struct metadata parsing for nested decimal fields #982

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3e28a25
fix: struct metadata parsing for nested decimal fields
sshpuntoff Sep 5, 2025
f6b4b63
test: minor test enhancements
sshpuntoff Sep 5, 2025
194335f
fix: remove un-used logger
sshpuntoff Sep 5, 2025
2c121b1
Merge branch 'main' into fix-decimal-parsing-in-struct-metadata
jayantsing-db Sep 9, 2025
193f9cd
next changelog
jayantsing-db Sep 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions src/main/java/com/databricks/jdbc/api/impl/MetadataParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,25 +94,33 @@ public static String parseMapMetadata(String metadata) {
* @return an array of field definitions in the STRUCT
*/
private static String[] splitFields(String metadata) {
int depth = 0;
int angleBracketDepth = 0;
int parenDepth = 0;
Copy link
Copy Markdown
Contributor Author

@sshpuntoff sshpuntoff Sep 5, 2025

Choose a reason for hiding this comment

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

there is a case to be made that just a single
int depth = 0; would work correctly here, and the code below just needs to be

if (ch == '<' || ch == '(') {
 depth ++;
}
else if (ch == '>' || ch == ')') {
    depth --;
}

Which would simplify the change set even more.
Let me know if you prefer a minimal diff over the explicit variable names.

StringBuilder currentField = new StringBuilder();
java.util.List<String> fields = new java.util.ArrayList<>();

for (char ch : metadata.toCharArray()) {
if (ch == '<') {
depth++;
angleBracketDepth++;
} else if (ch == '>') {
depth--;
angleBracketDepth--;
} else if (ch == '(') {
parenDepth++;
} else if (ch == ')') {
parenDepth--;
}

if (ch == ',' && depth == 0) {
fields.add(currentField.toString().trim());
// Only split on commas when we're at the top level (both depths are 0)
Comment thread
jayantsing-db marked this conversation as resolved.
if (ch == ',' && angleBracketDepth == 0 && parenDepth == 0) {
String field = currentField.toString().trim();
fields.add(field);
currentField.setLength(0);
} else {
currentField.append(ch);
}
}
fields.add(currentField.toString().trim());
String finalField = currentField.toString().trim();
fields.add(finalField);
return fields.toArray(new String[0]);
}

Expand Down
116 changes: 116 additions & 0 deletions src/test/java/com/databricks/jdbc/api/impl/MetadataParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,4 +292,120 @@ public void testParseMapMetadata_EmptyMap() {
exception.getMessage().contains("Invalid MAP metadata"),
"Exception message should indicate invalid MAP metadata.");
}

/**
* Test parsing of STRUCT metadata with DECIMAL precision and scale - regression test for
* parentheses handling.
*/
@Test
@DisplayName("parseStructMetadata with DECIMAL precision and scale")
public void testParseStructMetadata_WithDecimalPrecisionScale() {
String metadata = "STRUCT<name:STRING, amount:DECIMAL(18,2)>";
Map<String, String> expected = new LinkedHashMap<>();
expected.put("name", "STRING");
expected.put("amount", "DECIMAL(18,2)");

Map<String, String> actual = MetadataParser.parseStructMetadata(metadata);
assertEquals(
expected,
actual,
"Parsed struct metadata with DECIMAL precision and scale should handle parentheses correctly.");
}

/**
* Test parsing of STRUCT metadata with multiple DECIMAL fields with different precision/scale.
*/
@Test
@DisplayName("parseStructMetadata with multiple DECIMAL fields")
public void testParseStructMetadata_MultipleDecimalFields() {
String metadata = "STRUCT<id:INT, price:DECIMAL(10,2), amount:DECIMAL(18,4), name:STRING>";
Map<String, String> expected = new LinkedHashMap<>();
expected.put("id", "INT");
expected.put("price", "DECIMAL(10,2)");
expected.put("amount", "DECIMAL(18,4)");
expected.put("name", "STRING");

Map<String, String> actual = MetadataParser.parseStructMetadata(metadata);
assertEquals(
expected,
actual,
"Parsed struct metadata with multiple DECIMAL fields should handle all parentheses correctly.");
}

/** Test parsing of complex nested STRUCT with DECIMAL fields - comprehensive regression test. */
@Test
@DisplayName("parseStructMetadata with nested STRUCT containing DECIMAL")
public void testParseStructMetadata_NestedStructWithDecimal() {
String metadata =
"STRUCT<id:INT, financial:STRUCT<balance:DECIMAL(15,2), credit:DECIMAL(10,2)>, active:BOOLEAN>";
Map<String, String> expected = new LinkedHashMap<>();
expected.put("id", "INT");
expected.put("financial", "STRUCT<balance:DECIMAL(15,2), credit:DECIMAL(10,2)>");
expected.put("active", "BOOLEAN");

Map<String, String> actual = MetadataParser.parseStructMetadata(metadata);
assertEquals(
expected,
actual,
"Parsed struct metadata with nested STRUCT containing DECIMAL should preserve all type information.");
}

/** Test parsing of deeply nested STRUCT with multiple DECIMAL fields at different levels. */
@Test
@DisplayName("parseStructMetadata with deeply nested STRUCT and DECIMAL fields")
public void testParseStructMetadata_DeeplyNestedStructWithDecimals() {
String metadata =
"STRUCT<id:INT, account:STRUCT<balance:DECIMAL(18,4), details:STRUCT<fee:DECIMAL(5,2), rate:DECIMAL(10,6)>>, status:STRING>";
Map<String, String> expected = new LinkedHashMap<>();
expected.put("id", "INT");
expected.put(
"account",
"STRUCT<balance:DECIMAL(18,4), details:STRUCT<fee:DECIMAL(5,2), rate:DECIMAL(10,6)>>");
expected.put("status", "STRING");

Map<String, String> actual = MetadataParser.parseStructMetadata(metadata);
assertEquals(
expected,
actual,
"Parsed struct metadata with deeply nested STRUCT and DECIMAL fields should handle all levels correctly.");
}

/** Test parsing of STRUCT with mixed complex types and DECIMAL fields. */
@Test
@DisplayName("parseStructMetadata with mixed complex types and DECIMAL")
public void testParseStructMetadata_MixedComplexTypesWithDecimal() {
String metadata =
"STRUCT<id:INT, prices:ARRAY<DECIMAL(12,2)>, accounts:MAP<STRING, STRUCT<balance:DECIMAL(15,2)>>, summary:STRUCT<total:DECIMAL(20,4), count:INT>>";
Map<String, String> expected = new LinkedHashMap<>();
expected.put("id", "INT");
expected.put("prices", "ARRAY<DECIMAL(12,2)>");
expected.put("accounts", "MAP<STRING, STRUCT<balance:DECIMAL(15,2)>>");
expected.put("summary", "STRUCT<total:DECIMAL(20,4), count:INT>");

Map<String, String> actual = MetadataParser.parseStructMetadata(metadata);
assertEquals(
expected,
actual,
"Parsed struct metadata with mixed complex types and DECIMAL fields should handle all combinations correctly.");
}

/** Test parsing of STRUCT with other parenthesized types to ensure fix applies broadly. */
@Test
@DisplayName("parseStructMetadata with various parenthesized types")
public void testParseStructMetadata_VariousParenthesizedTypes() {
String metadata =
"STRUCT<id:INT, name:STRING, amount:DECIMAL(38,18), price:DECIMAL(10,2), active:BOOLEAN>";
Map<String, String> expected = new LinkedHashMap<>();
expected.put("id", "INT");
expected.put("name", "STRING");
expected.put("amount", "DECIMAL(38,18)");
expected.put("price", "DECIMAL(10,2)");
expected.put("active", "BOOLEAN");

Map<String, String> actual = MetadataParser.parseStructMetadata(metadata);
assertEquals(
expected,
actual,
"Parsed struct metadata with various parenthesized types should handle all type parameters correctly.");
}
}