Skip to content

Commit b91c988

Browse files
rustyconoverclaude
andcommitted
feat(catalog)!: rename required_field_filter_paths -> required_filters (CNF)
Port the breaking VGI wire-protocol change (vgi-python 0.15.0, vgi C++ extension) to the Go SDK. The TableInfo trailing field is renamed from required_field_filter_paths to required_filters, and its type changes from a flat AND-only []string to conjunctive normal form [][]string: an outer AND of inner OR-groups of dotted-path column references. A group is satisfied when any one of its member paths has a WHERE filter; every group must be satisfied. So [["accession_number"], ["ticker","cik"]] means "accession_number AND one of (ticker, cik)". No backward compatibility — the old name is removed entirely. - Wire type: list<utf8> -> list<list<utf8>> (arrow.ListOf(arrow.ListOf(String))), serialized via the nested list-builder pattern used by unique_constraints / primary_key_constraints. - TableInfo.RequiredFilters and CatalogTable.RequiredFilters are now [][]string. - Add validateRequiredFilters (mirrors vgi-python's descriptor): reject empty groups and empty strings; the leading dotted segment of each path must name a real column. - Convert example fixtures to singleton groups to preserve prior AND meaning and add rff_or, a genuine OR-group ([["a","b"]]) backing required_filters_or_group. - Add unit tests for CNF serialization round-trip and validation. Verified: go build ./..., go vet ./..., go test ./... (green), and the full Query-farm/vgi sqllogictest integration suite (make test: 9909 assertions, 249 cases, 0 failures), including required_filters_or_group. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent fb99a4c commit b91c988

7 files changed

Lines changed: 276 additions & 66 deletions

File tree

cmd/vgi-example-worker/main.go

Lines changed: 49 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -603,50 +603,60 @@ func main() {
603603
},
604604
StatisticsCacheMaxAgeSeconds: &statsTTL3600,
605605
})
606-
// required_field_filter_paths fixtures — back the
607-
// required_field_filter_paths_*.test sqllogictest matrix. The C++ optimizer
608-
// extension enforces the declared paths at bind time.
606+
// required_filters fixtures — back the required_filters_*.test
607+
// sqllogictest matrix. The C++ optimizer extension enforces the declared
608+
// CNF groups at bind time. Each group is an OR of member paths; every group
609+
// must be satisfied. Prior flat single-path requirements become singleton
610+
// groups to preserve their AND meaning.
609611
w.RegisterCatalogTable("data", vgi.CatalogTable{
610-
Name: "rff_simple",
611-
Comment: "rff_simple — requires a filter referencing column 'a'.",
612-
Columns: table.RffSimpleSchema,
613-
RequiredFieldFilterPaths: []string{"a"},
612+
Name: "rff_simple",
613+
Comment: "rff_simple — requires a filter referencing column 'a'.",
614+
Columns: table.RffSimpleSchema,
615+
RequiredFilters: [][]string{{"a"}},
614616
})
615617
w.RegisterCatalogTable("data", vgi.CatalogTable{
616-
Name: "rff_struct",
617-
Comment: "rff_struct — requires filters on both struct subfields s.a and s.b.",
618-
Columns: table.RffStructSchema,
619-
RequiredFieldFilterPaths: []string{"s.a", "s.b"},
618+
Name: "rff_struct",
619+
Comment: "rff_struct — requires filters on both struct subfields s.a and s.b.",
620+
Columns: table.RffStructSchema,
621+
RequiredFilters: [][]string{{"s.a"}, {"s.b"}},
620622
})
621623
w.RegisterCatalogTable("data", vgi.CatalogTable{
622-
Name: "rff_nested",
623-
Comment: "rff_nested — requires a filter on the 3-deep nested path wrapper.mid.leaf.",
624-
Columns: table.RffNestedSchema,
625-
RequiredFieldFilterPaths: []string{"wrapper.mid.leaf"},
624+
Name: "rff_nested",
625+
Comment: "rff_nested — requires a filter on the 3-deep nested path wrapper.mid.leaf.",
626+
Columns: table.RffNestedSchema,
627+
RequiredFilters: [][]string{{"wrapper.mid.leaf"}},
626628
})
627629
w.RegisterCatalogTable("data", vgi.CatalogTable{
628-
Name: "rff_multi",
629-
Comment: "rff_multi — mixed top-level + struct subfield requirements.",
630-
Columns: table.RffMultiSchema,
631-
RequiredFieldFilterPaths: []string{"top", "s.a"},
630+
Name: "rff_multi",
631+
Comment: "rff_multi — mixed top-level + struct subfield requirements (top AND s.a).",
632+
Columns: table.RffMultiSchema,
633+
RequiredFilters: [][]string{{"top"}, {"s.a"}},
634+
})
635+
// rff_or — genuine OR-group: a single CNF group satisfied by a filter on
636+
// EITHER a or b (i.e. [["a","b"]] means "one of (a, b)").
637+
w.RegisterCatalogTable("data", vgi.CatalogTable{
638+
Name: "rff_or",
639+
Comment: "rff_or — requires a filter on either column a OR column b (single OR-group).",
640+
Columns: table.RffNoneSchema,
641+
RequiredFilters: [][]string{{"a", "b"}},
632642
})
633643
w.RegisterCatalogTable("data", vgi.CatalogTable{
634644
Name: "rff_none",
635-
Comment: "rff_none — control table with no required_field_filter_paths (opt-out fast path).",
645+
Comment: "rff_none — control table with no required_filters (opt-out fast path).",
636646
Columns: table.RffNoneSchema,
637647
})
638648
// rff_rowid — row_id virtual column + required bbox.* filters. The rowid
639649
// table_filter is keyed by a sentinel >> column count, which the optimizer
640-
// must skip. See required_field_filter_paths_rowid.test.
650+
// must skip. See required_filters_rowid.test.
641651
w.RegisterCatalogTable("data", vgi.CatalogTable{
642-
Name: "rff_rowid",
643-
Comment: "rff_rowid — row_id virtual column + required bbox.* filters.",
644-
Columns: table.RffRowidSchema,
645-
RequiredFieldFilterPaths: []string{"bbox.xmin", "bbox.xmax", "bbox.ymin", "bbox.ymax"},
652+
Name: "rff_rowid",
653+
Comment: "rff_rowid — row_id virtual column + required bbox.* filters.",
654+
Columns: table.RffRowidSchema,
655+
RequiredFilters: [][]string{{"bbox.xmin"}, {"bbox.xmax"}, {"bbox.ymin"}, {"bbox.ymax"}},
646656
})
647657
// rff_parquet / rff_hive / rff_hive_mixed — native read_parquet delegation
648658
// with bbox.* required filters (mirrors Overture transportation.segment).
649-
// See required_field_filter_paths_native.test.
659+
// See required_filters_native.test.
650660
rffBboxType := arrow.StructOf(
651661
arrow.Field{Name: "xmin", Type: arrow.PrimitiveTypes.Float32},
652662
arrow.Field{Name: "ymin", Type: arrow.PrimitiveTypes.Float32},
@@ -660,7 +670,7 @@ func main() {
660670
{Name: "bbox", Type: rffBboxType},
661671
{Name: "other", Type: arrow.PrimitiveTypes.Int64},
662672
}, nil),
663-
RequiredFieldFilterPaths: []string{"bbox.xmin", "bbox.xmax", "bbox.ymin", "bbox.ymax"},
673+
RequiredFilters: [][]string{{"bbox.xmin"}, {"bbox.xmax"}, {"bbox.ymin"}, {"bbox.ymax"}},
664674
})
665675
rffHiveColumns := arrow.NewSchema([]arrow.Field{
666676
{Name: "id", Type: arrow.BinaryTypes.String},
@@ -671,16 +681,16 @@ func main() {
671681
{Name: "type", Type: arrow.BinaryTypes.String},
672682
}, nil)
673683
w.RegisterCatalogTable("data", vgi.CatalogTable{
674-
Name: "rff_hive",
675-
Comment: "rff_hive — native read_parquet over Hive glob with bbox.* required filters.",
676-
Columns: rffHiveColumns,
677-
RequiredFieldFilterPaths: []string{"bbox.xmin", "bbox.xmax", "bbox.ymin", "bbox.ymax"},
684+
Name: "rff_hive",
685+
Comment: "rff_hive — native read_parquet over Hive glob with bbox.* required filters.",
686+
Columns: rffHiveColumns,
687+
RequiredFilters: [][]string{{"bbox.xmin"}, {"bbox.xmax"}, {"bbox.ymin"}, {"bbox.ymax"}},
678688
})
679689
w.RegisterCatalogTable("data", vgi.CatalogTable{
680-
Name: "rff_hive_mixed",
681-
Comment: "rff_hive_mixed — native read_parquet, top-level 'id' + bbox.* required filters.",
682-
Columns: rffHiveColumns,
683-
RequiredFieldFilterPaths: []string{"id", "bbox.xmin", "bbox.xmax", "bbox.ymin", "bbox.ymax"},
690+
Name: "rff_hive_mixed",
691+
Comment: "rff_hive_mixed — native read_parquet, top-level 'id' + bbox.* required filters.",
692+
Columns: rffHiveColumns,
693+
RequiredFilters: [][]string{{"id"}, {"bbox.xmin"}, {"bbox.xmax"}, {"bbox.ymin"}, {"bbox.ymax"}},
684694
})
685695
// filter_echo_table — catalog table echoing pushed-down filters, backs
686696
// filter_pushdown_through_view.test.
@@ -946,6 +956,10 @@ func main() {
946956
return &vgi.ScanFunctionResult{FunctionName: "rff_multi_scan"}, nil
947957
case "rff_none":
948958
return &vgi.ScanFunctionResult{FunctionName: "rff_none_scan"}, nil
959+
case "rff_or":
960+
// rff_or reuses the (a, b) rff_none dataset; only its
961+
// required_filters (a single OR-group) differ.
962+
return &vgi.ScanFunctionResult{FunctionName: "rff_none_scan"}, nil
949963
case "rff_rowid":
950964
return &vgi.ScanFunctionResult{FunctionName: "rff_rowid_scan"}, nil
951965
case "filter_echo_table":

examples/all/all.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ func registerTables(w *vgi.Worker) {
191191
w.RegisterTable(table.NewProductsScanFunction())
192192
w.RegisterTable(table.NewProjectsScanFunction())
193193
// rff_* scan functions back the Tables exercised by the
194-
// required_field_filter_paths_*.test sqllogictest matrix.
194+
// required_filters_*.test sqllogictest matrix.
195195
w.RegisterTable(table.NewRffSimpleScanFunction())
196196
w.RegisterTable(table.NewRffStructScanFunction())
197197
w.RegisterTable(table.NewRffNestedScanFunction())

examples/table/required_filters.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@ import (
1414
)
1515

1616
// Scan functions backing the rff_* Tables exercised by the
17-
// required_field_filter_paths_*.test sqllogictest matrix. They mirror
17+
// required_filters_*.test sqllogictest matrix. They mirror
1818
// vgi-python's vgi/_test_fixtures/table/required_filters.py.
1919
//
20-
// The C++ VGI optimizer extension enforces Table.required_field_filter_paths
20+
// The C++ VGI optimizer extension enforces Table.required_filters
2121
// at bind/optimize time — rejecting any scan that lacks the declared WHERE
2222
// filters before a single worker byte is read. Once a scan passes that check,
2323
// DuckDB applies the actual filter itself, so these functions simply emit the
@@ -40,7 +40,7 @@ func (f *RffSimpleScanFunction) Name() string { return "rff_simple_scan" }
4040

4141
func (f *RffSimpleScanFunction) Metadata() vgi.FunctionMetadata {
4242
return vgi.FunctionMetadata{
43-
Description: "rff_simple — flat columns (a, b) for required_field_filter_paths tests",
43+
Description: "rff_simple — flat columns (a, b) for required_filters tests",
4444
Stability: vgi.StabilityConsistent,
4545
Categories: []string{"generator", "testing"},
4646
}
@@ -90,7 +90,7 @@ func (f *RffNoneScanFunction) Name() string { return "rff_none_scan" }
9090

9191
func (f *RffNoneScanFunction) Metadata() vgi.FunctionMetadata {
9292
return vgi.FunctionMetadata{
93-
Description: "rff_none — control table with no required_field_filter_paths",
93+
Description: "rff_none — control table with no required_filters",
9494
Stability: vgi.StabilityConsistent,
9595
Categories: []string{"generator", "testing"},
9696
}
@@ -145,7 +145,7 @@ func (f *RffStructScanFunction) Name() string { return "rff_struct_scan" }
145145

146146
func (f *RffStructScanFunction) Metadata() vgi.FunctionMetadata {
147147
return vgi.FunctionMetadata{
148-
Description: "rff_struct — STRUCT(s.a, s.b) + other for required_field_filter_paths tests",
148+
Description: "rff_struct — STRUCT(s.a, s.b) + other for required_filters tests",
149149
Stability: vgi.StabilityConsistent,
150150
Categories: []string{"generator", "testing"},
151151
}
@@ -216,7 +216,7 @@ func (f *RffNestedScanFunction) Name() string { return "rff_nested_scan" }
216216

217217
func (f *RffNestedScanFunction) Metadata() vgi.FunctionMetadata {
218218
return vgi.FunctionMetadata{
219-
Description: "rff_nested — nested STRUCT(wrapper.mid.leaf) for required_field_filter_paths tests",
219+
Description: "rff_nested — nested STRUCT(wrapper.mid.leaf) for required_filters tests",
220220
Stability: vgi.StabilityConsistent,
221221
Categories: []string{"generator", "testing"},
222222
}
@@ -339,7 +339,7 @@ func NewRffMultiScanFunction() vgi.TableFunction {
339339
// Because of the virtual rowid column this fixture needs projection_pushdown:
340340
// under projection the emitted batch must match the *projected* output schema,
341341
// so it builds only the requested columns. See
342-
// required_field_filter_paths_rowid.test.
342+
// required_filters_rowid.test.
343343
// ---------------------------------------------------------------------------
344344

345345
// rffRowidMetadata marks the row_id virtual column (hidden from SELECT *).

vgi/catalog.go

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"bytes"
77
"context"
88
"fmt"
9+
"strings"
910

1011
"github.com/Query-farm/vgi-rpc-go/vgirpc"
1112
"github.com/apache/arrow-go/v18/arrow"
@@ -1775,6 +1776,43 @@ func resolveColumnGroupIndices(columns *arrow.Schema, groups [][]string) [][]int
17751776
return result
17761777
}
17771778

1779+
// validateRequiredFilters checks a table's CNF required-filter groups. Each
1780+
// OR-group must be non-empty and contain no empty strings, and the leading
1781+
// dotted segment of every path must name a real column on the table. Returns
1782+
// nil when groups is empty (the no-enforcement fast path) or columns is nil.
1783+
func validateRequiredFilters(tableName string, columns *arrow.Schema, groups [][]string) error {
1784+
if len(groups) == 0 {
1785+
return nil
1786+
}
1787+
columnNames := map[string]struct{}{}
1788+
if columns != nil {
1789+
for i := 0; i < columns.NumFields(); i++ {
1790+
columnNames[columns.Field(i).Name] = struct{}{}
1791+
}
1792+
}
1793+
for _, group := range groups {
1794+
if len(group) == 0 {
1795+
return fmt.Errorf("table %q: required_filters must not contain empty groups", tableName)
1796+
}
1797+
for _, path := range group {
1798+
if path == "" {
1799+
return fmt.Errorf("table %q: required_filters must not contain empty strings", tableName)
1800+
}
1801+
if columns == nil {
1802+
continue
1803+
}
1804+
head := path
1805+
if idx := strings.IndexByte(path, '.'); idx >= 0 {
1806+
head = path[:idx]
1807+
}
1808+
if _, ok := columnNames[head]; !ok {
1809+
return fmt.Errorf("table %q: required_filters path %q references unknown column %q", tableName, path, head)
1810+
}
1811+
}
1812+
}
1813+
return nil
1814+
}
1815+
17781816
// serializeCatalogTable converts a CatalogTable into serialized TableInfo bytes.
17791817
func (w *Worker) serializeCatalogTable(schemaName string, ct *CatalogTable) ([]byte, error) {
17801818
// Resolve columns: if Function is set but Columns is nil, derive via OnBind
@@ -1824,6 +1862,14 @@ func (w *Worker) serializeCatalogTable(schemaName string, ct *CatalogTable) ([]b
18241862
unique := resolveColumnGroupIndices(columns, ct.Unique)
18251863
primaryKey := resolveColumnGroupIndices(columns, ct.PrimaryKey)
18261864

1865+
// Validate required_filters (CNF): each OR-group must be non-empty and
1866+
// contain no empty strings, and the leading dotted segment of each path
1867+
// must be a real column on this table. Mirrors vgi-python's descriptor
1868+
// validation; struct-subfield validity is left to DuckDB's binder.
1869+
if err := validateRequiredFilters(ct.Name, columns, ct.RequiredFilters); err != nil {
1870+
return nil, err
1871+
}
1872+
18271873
// Serialize FOREIGN KEY constraints
18281874
var foreignKeys [][]byte
18291875
for _, fk := range ct.ForeignKey {
@@ -1848,7 +1894,7 @@ func (w *Worker) serializeCatalogTable(schemaName string, ct *CatalogTable) ([]b
18481894
SupportsColumnStatistics: len(ct.Statistics) > 0,
18491895
CardinalityEstimate: ct.CardinalityEstimate,
18501896
CardinalityMax: ct.CardinalityMax,
1851-
RequiredFieldFilterPaths: ct.RequiredFieldFilterPaths,
1897+
RequiredFilters: ct.RequiredFilters,
18521898
}
18531899

18541900
// Inline the scan function for function-backed tables so the C++ extension

0 commit comments

Comments
 (0)