Reduce the generator to a stock serializer, remove the migration flag, and document the extension hook — Closes #146#168
Merged
Conversation
Every GIQL operator now expands to standard AST in the normalization passes, so the custom generator earns its keep no longer. Delete the giql.generators package and BaseGIQLGenerator and serialize the final AST with the stock sqlglot serializer selected by the active target's sqlglot_dialect. The five NEAREST helpers and the distance-CASE string builder move out of the generator into the expander modules. Remove the GIQL_EXPAND migration flag. With every operator migrated the per-node opt-in gate is dead weight, so it and the legacy emit path are gone; a resolve miss now raises a clear error instead of falling back to a legacy emitter that no longer exists. Make the expander registry a target plugin hub. A custom Target can be declared with register_target, or as a side effect of register, and selected with transpile(dialect=name), so the registry becomes a supported public extension point. resolve_target consults the registry for a non-built-in name while built-in names still win. Export the hook symbols from the top-level giql package and document the extension point with a runnable worked example. Serialization is byte-identical to the previous generator for the generic and DataFusion targets. The only change is that the DuckDB target now makes DuckDB's default NULLS FIRST ordering explicit. Claude-Session: https://claude.ai/code/session_01ALxmQysPad4W68wuWuft6W
9cf1ed4 to
3b92c43
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Complete epic #137. With every GIQL operator migrated onto the
ExpandOperatorsAST pass and its(target, operator)registry, the transpiler no longer needs a custom generator: reduce it to stock per-target sqlglot serialization, remove the now-deadGIQL_EXPANDmigration flag and legacy emit path, and promote the expander registry to a supported public extension point for custom targets and operator overrides.The custom
BaseGIQLGeneratorand thegiql.generatorspackage are deleted; the final AST is serialized with the stock sqlglot serializer selected by the active target'ssqlglot_dialect. The five NEAREST helpers and the distance-CASE string builder move out of the generator into the expander modules. TheExpanderRegistrybecomes a target plugin hub — a customTargetis declared viaregister_target(or as a side effect ofregister) and selected withtranspile(dialect="<name>")— and the hook symbols are exported from the top-levelgiqlpackage and documented with a runnable worked example.Trade-off worth noting: serialization is byte-identical to the previous generator for the generic and DataFusion targets; the only change is that the DuckDB target now makes DuckDB's default
NULLS FIRSTordering explicit. The query-level expander seam for whole-query rewrites (the deferred INTERSECTS IEJoin fold and the NEARESTSELECT *column leak, #160) is intentionally left as follow-up.Closes #146
Proposed changes
Reduce the generator to a stock serializer
Delete
src/giql/generators/(BaseGIQLGenerator+ package) and serialize the expanded AST intranspilewithast.sql(dialect=target.sqlglot_dialect). Relocate the six former generator helpers out of the generator: the five NEAREST-only helpers (_nearest_output_encoding,_nearest_passthrough,_detect_nearest_mode,_raise_nearest_reference_error,_extract_bool_param) become module functions ingiql.expanders.nearest, and the shared distance-CASE string builder moves to a newgiql.expanders._distance.generate_distance_case.Remove the GIQL_EXPAND flag and the legacy emit path
Drop the per-operator
GIQL_EXPANDopt-in flag from all nine operator classes and the gate in theExpandOperatorspass. Every operator now expands unconditionally; a resolve miss raises a clearValueErrornaming the operator and target instead of falling back to a legacy*_sqlemitter that no longer exists.Make the expander registry a target plugin hub
Add
ExpanderRegistry.register_targetandtarget, haveregisterdeclare its target as a side effect, and extendclear/snapshot/restore(now via aRegistrySnapshotvalue type) to cover declared targets.resolve_targetconsults the registry for a non-built-in dialect name while built-in names still win, sotranspile(dialect="<custom>")resolves a registered custom target end to end.Export and document the extension hook
Re-export
register,REGISTRY,ExpanderRegistry,ExpansionContext,OperatorExpander,Target,Capabilities, and the three built-in targets from the top-levelgiqlpackage. Adddocs/transpilation/extending.rstwith runnable custom-target and operator-override examples plus the node-local boundary and the pre-quoted-identifier caveat; extend the API reference and cross-link schema-mapping.Test cases
TestPublicApiSurfacegiql.__all__giqlroot and compared to its submodule originTestPublicApiSurfacegiql.generatorspackageimport giql.generatorsModuleNotFoundErrorTestCustomTargetInjectionregister_targettranspile(dialect="postgres")resolve_targetregistry fallbackTestCustomTargetInjectionWithinexpander overrideTestCustomTargetInjectionresolve_target("duckdb")TestTargetDrivesSerializationsqlglot_dialect="duckdb"NULLS FIRSTsqlglot_dialectthreadingTestExpandOperatorsPassValueErrornaming the operator and targetTestRegistrySnapshot==RegistrySnapshotequalityTestTargetRegistrationTestTranspileDialectsTestTranspileDialectsNULLS FIRSTis the sole DuckDB deltaTestDistanceExpanderStringParityexpand_distance(AST) andgenerate_distance_case(string) evaluated in DuckDBTestNearestDataFusionFallbackShapemax_distanceon DataFusion