fix for STRUCT() with unknown fields in duckdb (#2094)

There's a blog post with an example where a STRUCT(x UUID) field exists and the type parser for duckdb did not properly handle that. Fixed the bug and added the missing test that should have existed.
malloydata · Jan 13, 2025 · eb6f3c2 · eb6f3c2
1 parent 8216876
commit eb6f3c2
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 12 deletions.
diff --git a/packages/malloy-db-duckdb/src/duckdb.spec.ts b/packages/malloy-db-duckdb/src/duckdb.spec.ts
@@ -167,6 +167,22 @@ describe('DuckDBConnection', () => {
         ],
       });
     });
+    it('parses struct with sql native field', () => {
+      const structDef = makeStructDef();
+      connection.fillStructDefFromTypeMap(structDef, {test: PROFESSOR_SCHEMA});
+      expect(structDef.fields[0]).toEqual({
+        'name': 'test',
+        'type': 'array',
+        'elementTypeDef': {type: 'record_element'},
+        'join': 'many',
+        'fields': [
+          {'name': 'professor_id', 'type': 'sql native', 'rawType': 'UUID'},
+          {'name': 'name', 'type': 'string'},
+          {'name': 'age', 'numberType': 'integer', 'type': 'number'},
+          {'name': 'total_sections', 'numberType': 'integer', 'type': 'number'},
+        ],
+      });
+    });
 
     it('parses a simple type', () => {
       const structDef = makeStructDef();
@@ -176,6 +192,16 @@ describe('DuckDBConnection', () => {
         'type': 'string',
       });
     });
+
+    it('parses unknown type', () => {
+      const structDef = makeStructDef();
+      connection.fillStructDefFromTypeMap(structDef, {test: 'UUID'});
+      expect(structDef.fields[0]).toEqual({
+        'name': 'test',
+        'type': 'sql native',
+        'rawType': 'UUID',
+      });
+    });
   });
 });
 
@@ -261,3 +287,6 @@ const NESTED_SCHEMA = 'STRUCT(a double, b integer, c varchar(60))[]';
 const intTyp = {type: 'number', numberType: 'integer'};
 const strTyp = {type: 'string'};
 const dblType = {type: 'number', numberType: 'float'};
+
+const PROFESSOR_SCHEMA =
+  'STRUCT(professor_id UUID, "name" VARCHAR, age BIGINT, total_sections BIGINT)[]';
diff --git a/packages/malloy/src/dialect/duckdb/duckdb.ts b/packages/malloy/src/dialect/duckdb/duckdb.ts
@@ -488,7 +488,6 @@ class DuckDBTypeParser extends TinyParser {
   }
 
   typeDef(): AtomicTypeDef {
-    const unknownStart = this.parseCursor;
     const wantID = this.next('id');
     const id = this.sqlID(wantID);
     let baseType: AtomicTypeDef;
@@ -536,20 +535,18 @@ class DuckDBTypeParser extends TinyParser {
       }
     } else {
       if (wantID.type === 'id') {
-        for (;;) {
-          const next = this.peek();
-          // Might be WEIRDTYP(a,b)[] ... stop at the []
-          if (next.type === 'arrayOf' || next.type === 'eof') {
-            break;
-          }
+        // unknown field type, strip all type decorations, there was a regex for this
+        // in the pre-parser code, but no tests, so this is also untested
+        let idEnd = wantID.cursor + wantID.text.length;
+        if (this.peek().type === 'precision') {
           this.next();
         }
+        if (this.peek().type === 'eof') {
+          idEnd = this.input.length;
+        }
         baseType = {
           type: 'sql native',
-          rawType: this.input.slice(
-            unknownStart,
-            this.parseCursor - unknownStart + 1
-          ),
+          rawType: this.input.slice(wantID.cursor, idEnd),
         };
       } else {
         throw this.parseError('Could not understand type');

diff --git a/packages/malloy/src/dialect/tiny_parser.ts b/packages/malloy/src/dialect/tiny_parser.ts
@@ -6,6 +6,7 @@
  */
 
 export interface TinyToken {
+  cursor: number;
   type: string;
   text: string;
 }
@@ -139,12 +140,14 @@ export class TinyParser {
         if (foundToken) {
           notFound = false;
           let tokenText = foundToken[0];
+          const cursor = this.parseCursor;
           this.parseCursor += tokenText.length;
           if (tokenType !== 'space') {
             if (tokenType[0] === 'q') {
               tokenText = tokenText.slice(1, -1); // strip quotes
             }
             yield {
+              cursor,
               type: tokenType === 'char' ? tokenText : tokenType,
               text: tokenText,
             };
@@ -153,7 +156,7 @@ export class TinyParser {
         }
       }
       if (notFound) {
-        yield {type: 'unexpected token', text: src};
+        yield {cursor: this.parseCursor, type: 'unexpected token', text: src};
         return;
       }
     }