Skip to content

Commit f471996

Browse files
pg_lake_engine: extract ConvertTypeTree generic type-tree walker
MaybeConvertType (unsupported numeric -> double) hard-coded its own recursion through arrays, maps, domains, and composites. Pull that structural traversal into a single ConvertTypeTree walker parameterized by a TypeLeafConverter callback (with nesting level + context), so future passes can reuse one authoritative traversal instead of copying it. MaybeConvertType becomes a thin wrapper supplying the numeric->float8 leaf rule; behavior is unchanged and the existing numeric tests still pass. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 45e1225 commit f471996

2 files changed

Lines changed: 143 additions & 53 deletions

File tree

pg_lake_engine/include/pg_lake/util/rel_utils.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,15 @@ extern PGDLLEXPORT void ErrorIfTypeUnsupportedForIcebergTables(Oid typeOid, int3
4646
extern PGDLLEXPORT void ErrorIfTypeUnsupportedNumericForIcebergTables(int32 typmod, char *columnName);
4747
extern PGDLLEXPORT void MaybeConvertUnsupportedNumericColumnsToDouble(List *columnDefList);
4848
extern PGDLLEXPORT PGType MaybeConvertType(PGType type, char *columnName);
49+
50+
/* leaf rule for ConvertTypeTree; see rel_utils.c */
51+
typedef bool (*TypeLeafConverter) (Oid typeOid, int32 typeMod, int level,
52+
void *context,
53+
Oid *outOid, int32 *outMod);
54+
55+
extern PGDLLEXPORT bool ConvertTypeTree(Oid typeOid, int32 typeMod, int level,
56+
TypeLeafConverter leafConv, void *context,
57+
Oid *outTypeOid, int32 *outTypeMod);
4958
extern PGDLLEXPORT PgLakeTableProperties GetPgLakeTableProperties(Oid relationId);
5059

5160
/* range var help */

pg_lake_engine/src/utils/rel_utils.c

Lines changed: 134 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -439,73 +439,112 @@ FindOrCreateCompositeTypeFromColumnDefs(List *coldeflist)
439439

440440

441441
/*
442-
* MaybeConvertType recursively converts a type that contains unsupported
443-
* numerics. Returns a PGType with the replacement OID, or with InvalidOid
444-
* when no conversion is needed.
442+
* ConvertTypeTree recursively rewrites a (typeOid, typeMod) by applying a
443+
* caller-supplied leaf rule to every scalar leaf, while handling the container
444+
* structure (array / map / domain / composite) itself. Returns true and fills
445+
* *outTypeOid / *outTypeMod when anything was rewritten; false (outputs
446+
* untouched) otherwise.
447+
*
448+
* The caller supplies the leaf rule via a TypeLeafConverter callback, which is
449+
* invoked on every node and must return false for container types (arrays,
450+
* maps, domains, composites) -- those are handled structurally here; returning
451+
* true and filling *outOid / *outMod requests a rewrite of that scalar leaf.
452+
* `level` is 0 for a top-level table column and increments by one for every
453+
* array element, map key/value, or composite field descended into; `context`
454+
* is passed through untouched.
455+
*
456+
* This is the single place that knows how pg_lake types nest, so independent
457+
* passes (unsupported numeric -> double, snowflake compatibility, ...) share
458+
* one structural traversal and cannot drift out of coverage. The container
459+
* rules are:
460+
*
461+
* array of X -> array of ConvertTypeTree(X) at level + 1
462+
* map (domain over array) -> new map via GetOrCreatePGMapType, key/value
463+
* converted at level + 1
464+
* domain (non-map) -> unwrap and recurse into base at same level
465+
* composite containing any -> new composite via
466+
* FindOrCreateCompositeTypeFromColumnDefs,
467+
* fields converted at level + 1, dropped
468+
* attributes skipped
445469
*
446-
* numeric (unsupported) -> FLOAT8OID
447-
* array of X -> array of MaybeConvertType(X)
448-
* map (domain over array) -> new map via GetOrCreatePGMapType
449-
* domain (non-map) -> unwrap and recurse into base type
450-
* composite containing any -> new composite via GetOrCreatePGStructType
470+
* User-defined composite/map types are never mutated: a fresh type is created
471+
* whenever a nested field changes.
451472
*/
452-
PGType
453-
MaybeConvertType(PGType type, char *columnName)
473+
bool
474+
ConvertTypeTree(Oid typeOid, int32 typeMod, int level,
475+
TypeLeafConverter leafConv, void *context,
476+
Oid *outTypeOid, int32 *outTypeMod)
454477
{
455-
Oid typeOid = type.postgresTypeOid;
456-
int32 typmod = type.postgresTypeMod;
457-
458-
/* unsupported numeric -> float8 */
459-
if (IsUnsupportedNumericForIceberg(typeOid, typmod))
460-
return MakePGTypeOid(FLOAT8OID);
478+
/* leaf rule first; the callback returns false for container types */
479+
if (leafConv(typeOid, typeMod, level, context, outTypeOid, outTypeMod))
480+
return true;
461481

462-
/* array: recurse into element type */
482+
/* array: recurse into the element type one level deeper */
463483
Oid elemType = get_element_type(typeOid);
464484

465485
if (OidIsValid(elemType))
466486
{
467-
PGType converted = MaybeConvertType(MakePGType(elemType, typmod),
468-
columnName);
487+
Oid rewrittenElementOid;
488+
int32 rewrittenElementMod;
469489

470-
if (OidIsValid(converted.postgresTypeOid))
471-
return MakePGTypeOid(get_array_type(converted.postgresTypeOid));
472-
return MakePGTypeOid(InvalidOid);
490+
if (ConvertTypeTree(elemType, typeMod, level + 1, leafConv, context,
491+
&rewrittenElementOid, &rewrittenElementMod))
492+
{
493+
*outTypeOid = get_array_type(rewrittenElementOid);
494+
*outTypeMod = -1;
495+
return true;
496+
}
497+
498+
return false;
473499
}
474500

475501
/* map check must precede the generic domain unwrap (maps are domains) */
476502
if (IsMapTypeOid(typeOid))
477503
{
478-
PGType keyType = GetMapKeyType(typeOid);
479-
PGType valType = GetMapValueType(typeOid);
480-
PGType newKey = MaybeConvertType(keyType, columnName);
481-
PGType newVal = MaybeConvertType(valType, columnName);
482-
483-
if (!OidIsValid(newKey.postgresTypeOid) &&
484-
!OidIsValid(newVal.postgresTypeOid))
485-
return MakePGTypeOid(InvalidOid);
486-
487-
Oid finalKeyOid = OidIsValid(newKey.postgresTypeOid) ? newKey.postgresTypeOid : keyType.postgresTypeOid;
488-
Oid finalValOid = OidIsValid(newVal.postgresTypeOid) ? newVal.postgresTypeOid : valType.postgresTypeOid;
504+
PGType origKeyType = GetMapKeyType(typeOid);
505+
PGType origValueType = GetMapValueType(typeOid);
506+
Oid rewrittenKeyOid;
507+
int32 rewrittenKeyMod;
508+
Oid rewrittenValueOid;
509+
int32 rewrittenValueMod;
510+
511+
bool keyRewritten = ConvertTypeTree(origKeyType.postgresTypeOid,
512+
origKeyType.postgresTypeMod,
513+
level + 1, leafConv, context,
514+
&rewrittenKeyOid, &rewrittenKeyMod);
515+
bool valueRewritten = ConvertTypeTree(origValueType.postgresTypeOid,
516+
origValueType.postgresTypeMod,
517+
level + 1, leafConv, context,
518+
&rewrittenValueOid, &rewrittenValueMod);
519+
520+
if (!keyRewritten && !valueRewritten)
521+
return false;
522+
523+
/* keep the original type for whichever side the leaf rule left alone */
524+
Oid newMapKeyOid = keyRewritten ? rewrittenKeyOid : origKeyType.postgresTypeOid;
525+
Oid newMapValueOid = valueRewritten ? rewrittenValueOid : origValueType.postgresTypeOid;
489526
const char *mapTypeName = psprintf("MAP(%s,%s)",
490-
GetFullDuckDBTypeNameForPGType(MakePGTypeOid(finalKeyOid), DATA_FORMAT_ICEBERG),
491-
GetFullDuckDBTypeNameForPGType(MakePGTypeOid(finalValOid), DATA_FORMAT_ICEBERG));
492-
493-
Oid mapOid = GetOrCreatePGMapType(mapTypeName);
527+
GetFullDuckDBTypeNameForPGType(MakePGTypeOid(newMapKeyOid), DATA_FORMAT_ICEBERG),
528+
GetFullDuckDBTypeNameForPGType(MakePGTypeOid(newMapValueOid), DATA_FORMAT_ICEBERG));
494529

495-
return MakePGTypeOid(mapOid);
530+
*outTypeOid = GetOrCreatePGMapType(mapTypeName);
531+
*outTypeMod = -1;
532+
return true;
496533
}
497534

498535
char typeType = get_typtype(typeOid);
499536

500-
/* domain (non-map): unwrap and recurse */
537+
/* domain (non-map): unwrap and recurse at the same level */
501538
if (typeType == TYPTYPE_DOMAIN)
502539
{
503-
Oid baseType = getBaseTypeAndTypmod(typeOid, &typmod);
540+
int32 baseMod = typeMod;
541+
Oid baseType = getBaseTypeAndTypmod(typeOid, &baseMod);
504542

505-
return MaybeConvertType(MakePGType(baseType, typmod), columnName);
543+
return ConvertTypeTree(baseType, baseMod, level, leafConv, context,
544+
outTypeOid, outTypeMod);
506545
}
507546

508-
/* composite: check each attribute */
547+
/* composite: rebuild with rewritten fields if any field changes */
509548
if (typeType == TYPTYPE_COMPOSITE)
510549
{
511550
TupleDesc tupdesc = lookup_rowtype_tupdesc(typeOid, -1);
@@ -516,39 +555,81 @@ MaybeConvertType(PGType type, char *columnName)
516555
{
517556
Form_pg_attribute attr = TupleDescAttr(tupdesc, i);
518557

558+
/* dropped columns must be ignored, never converted or re-added */
519559
if (attr->attisdropped)
520560
continue;
521561

522-
Oid targetType = attr->atttypid;
523-
int32 targetTypmod = attr->atttypmod;
524-
PGType converted = MaybeConvertType(
525-
MakePGType(targetType, targetTypmod), columnName);
562+
Oid fieldType = attr->atttypid;
563+
int32 fieldMod = attr->atttypmod;
564+
Oid rewrittenFieldOid;
565+
int32 rewrittenFieldMod;
526566

527-
if (OidIsValid(converted.postgresTypeOid))
567+
if (ConvertTypeTree(fieldType, fieldMod, level + 1, leafConv, context,
568+
&rewrittenFieldOid, &rewrittenFieldMod))
528569
{
529-
targetType = converted.postgresTypeOid;
530-
targetTypmod = converted.postgresTypeMod;
570+
fieldType = rewrittenFieldOid;
571+
fieldMod = rewrittenFieldMod;
531572
needsConversion = true;
532573
}
533574

534575
ColumnDef *colDef = makeNode(ColumnDef);
535576

536577
colDef->colname = pstrdup(NameStr(attr->attname));
537-
colDef->typeName = makeTypeNameFromOid(targetType, targetTypmod);
578+
colDef->typeName = makeTypeNameFromOid(fieldType, fieldMod);
538579
colDef->is_local = true;
539580
coldeflist = lappend(coldeflist, colDef);
540581
}
541582

542583
ReleaseTupleDesc(tupdesc);
543584

544585
if (!needsConversion)
545-
return MakePGTypeOid(InvalidOid);
586+
return false;
587+
588+
*outTypeOid = FindOrCreateCompositeTypeFromColumnDefs(coldeflist);
589+
*outTypeMod = -1;
590+
return true;
591+
}
592+
593+
return false;
594+
}
546595

547-
Oid compositeOid = FindOrCreateCompositeTypeFromColumnDefs(coldeflist);
548596

549-
return MakePGTypeOid(compositeOid);
597+
/*
598+
* NumericLeafToDouble is the ConvertTypeTree leaf rule for the unsupported
599+
* numeric -> float8 pass. Numeric is never a container, so it applies at any
600+
* nesting level; level and context are unused.
601+
*/
602+
static bool
603+
NumericLeafToDouble(Oid typeOid, int32 typeMod, int level, void *context,
604+
Oid *outOid, int32 *outMod)
605+
{
606+
if (IsUnsupportedNumericForIceberg(typeOid, typeMod))
607+
{
608+
*outOid = FLOAT8OID;
609+
*outMod = -1;
610+
return true;
550611
}
551612

613+
return false;
614+
}
615+
616+
617+
/*
618+
* MaybeConvertType recursively converts a type that contains unsupported
619+
* numerics. Returns a PGType with the replacement OID, or with InvalidOid
620+
* when no conversion is needed. Thin wrapper over ConvertTypeTree with the
621+
* numeric leaf rule; columnName is retained for call-site readability.
622+
*/
623+
PGType
624+
MaybeConvertType(PGType type, char *columnName)
625+
{
626+
Oid convOid;
627+
int32 convMod;
628+
629+
if (ConvertTypeTree(type.postgresTypeOid, type.postgresTypeMod, 0,
630+
NumericLeafToDouble, NULL, &convOid, &convMod))
631+
return MakePGType(convOid, convMod);
632+
552633
return MakePGTypeOid(InvalidOid);
553634
}
554635

0 commit comments

Comments
 (0)