Repository navigation
Conversation
Co-authored-by: Codex GPT-5 <codex@openai.com>
Co-authored-by: Codex GPT-5 <codex@openai.com>
Co-authored-by: Codex GPT-5 <codex@openai.com>
Co-authored-by: Codex GPT-5 <codex@openai.com>
Co-authored-by: Codex GPT-5 <codex@openai.com>
Remove the unused SQL function provider from union-step construction so the compiler IR no longer carries a false runtime dependency. Also replace Javadoc-only links to select DTOs with IR terminology. Co-authored-by: Codex GPT-5 <codex@openai.com>
Make dialect-specific final select aggregation part of CompilerDialect and bridge the legacy backend provider to the new contract. Add jOOQ to the connector module so compiler IR can depend directly on SQL expression types. Co-authored-by: Codex GPT-5 <codex@openai.com>
Add a self-contained compiled query with an ordered physical column layout so backends can execute SQL and correlate results without compiler-owned result processing. Replace presentation metadata in resolved result columns with stable output identifiers and validate identifiers and SQL aliases for uniqueness. Co-authored-by: Codex GPT-5 <codex@openai.com>
Add a backend-neutral SqlCompiler that compiles a resolved query for an explicitly selected dialect. Remove execution routing from ResolvedQuery so dataset revisions and datasource selection remain backend responsibilities. Co-authored-by: Codex GPT-5 <codex@openai.com>
Move the qualification and select intermediate-representation contracts into sql-connector so compiler transformations no longer depend on backend-owned types. Update the legacy backend to consume the connector contracts. Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Move the field-backed, extracting, and existence select representations into the connector-owned compiler IR. Use plain Java implementations, preserve the legacy equality behavior, and cover the relocated transformations with tests. Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Move the two-column date range representation and field qualification utility into the connector-owned compiler IR. Replace Lombok-generated accessors with plain Java and cover qualification, aliasing, and coalescing behavior with tests. Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Move shared compiler aliases and plain and stratified ID-column representations into the connector-owned IR. Normalize backend resolution enums before crossing the boundary and cover aliasing, qualification, and stratification with tests. Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Move the Selects aggregate and its pure transformations into the connector-owned IR while preserving its Lombok value and builder API. Keep dialect-aware final projection rendering in the backend transformer and cover the moved aggregate with focused tests. Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Move dialect-aware Selects rendering into the connector compiler. Expose physical date-range rendering through CompilerDialect, bridge the legacy function provider, and cover aggregated and individual validity-date modes. Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Move QueryStep into the connector-owned IR and rewire legacy compiler consumers. Replace the backend-specific table-export flag with an explicit projection mode and cover qualification, unions, and final rendering modes. Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Move recursive CTE, grouping, and union rendering into the connector compiler. Keep backend integration limited to constructing and invoking the renderer, and cover predecessor and union rendering with focused connector tests. Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Move join modes, table joining, ID coalescing, and select merging into the connector compiler IR. Rename the remaining backend helper to QueryStepComposer and cover portable join construction with focused connector tests. Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Move DateAggregationDates into the connector compiler IR as a qualifiable value object. Rewire the legacy aggregation pipeline and cover filtering, query-step qualification, selected bounds, and empty inputs with focused tests. Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Move the pure date-propagation vocabulary into the dependency-free shared model so the legacy and SQL query models use one enum. Keep CDateSet operations in the backend DateAggregator and preserve their behavior with focused tests. Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Move CTE step and table-resolution structures into the connector compiler IR. Decouple CTE name mapping from the backend name generator through a naming function and cover predecessor, naming, and table resolution behavior. Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Move stateful SQL alias and CTE naming into the connector compiler. Add stable technical names to resolved selects and filters, expose typed naming methods, and retain explicit legacy bridges until backend conversion uses this model. Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Move composable WHERE conditions and phase-specific clause grouping out of the legacy backend. Keep backend conversion on connector-owned IR and cover logical composition, inversion, flag matching, and clause grouping. Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Move string membership rendering into the connector's condition IR. Remove the unused legacy dialect indirection, align the internal name with the resolved model, and cover single, multiple, empty, null, and negated selections. Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Centralize strict date-range overlap rendering in the connector's condition IR. Remove identical HANA and ClickHouse dialect hooks, migrate legacy callers, and cover the exclusive-end overlap expression. Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Centralize inclusive-bound rendering in the connector's condition IR. Introduce a shared, order-validating range contract implemented by both resolved and legacy models, then remove the remaining backend condition utility. Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Replace the type-named backend range condition wrappers with one explicit compatibility adapter. Keep the wildcard bridge isolated until backend conversion produces connector model ranges directly. Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Move dialect-independent field expressions into connector IR and expose date sentinels through CompilerDialect. Replace DateAggregationContext's broad backend conversion state with the narrow compiler dialect dependency. Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Move the framework-neutral interval-packing transformation and its CTE graph into the connector IR. Remove the unused PostgreSQL strategy and obsolete dialect plumbing now that HANA and ClickHouse share the ANSI implementation. Cover both multi-step and single-column CTE mappings and fix the latter to reference the root table when intermediate steps are omitted. Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Move the date aggregation and inversion CTE graph into the connector IR and expose it through a framework-neutral DateAggregationCompiler. Retain a small legacy backend adapter that supplies dialect and naming dependencies. Add direct connector tests for merge, intersect, inversion, empty date input, and unsupported aggregation actions. Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Convert resolved string-value and numeric-range filters into event-level connector predicates. Preserve zero-based substring semantics when building SQL expressions and provide the compiler context and dispatcher boundary for extension-defined filter converters. Document the preprocessing, event, and post-aggregation condition buckets consumed by the connector pipeline. Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Convert resolved flag and date-distance filters into event-level connector predicates. Keep range and flag semantics in the connector while delegating database-specific date-distance expressions to the compiler dialect and preserving the legacy backend adapter. Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Co-authored-by: Codex GPT-6 <codex@openai.com>
Preserve develop SQL semantics for subtraction, distinct keys, numeric types, and date ranges. Record local compatibility coverage and the remaining extraction plan. Co-authored-by: Codex GPT-6 <codex@openai.com>
Co-authored-by: Codex GPT-6 <codex@openai.com>
Co-authored-by: Codex GPT-6 <codex@openai.com>
Co-authored-by: Codex GPT-6 <codex@openai.com>
Co-authored-by: Codex GPT-6 <codex@openai.com>
Co-authored-by: Codex GPT-6 <codex@openai.com>
Co-authored-by: Codex GPT-6 <codex@openai.com>
Co-authored-by: Codex GPT-6 <codex@openai.com>
Co-authored-by: Codex GPT-5.6-sol <codex@openai.com>
Co-authored-by: Codex GPT-6 <codex@openai.com>
Co-authored-by: Codex GPT-6 <codex@openai.com>
Co-authored-by: Codex GPT-6 <codex@openai.com>
Co-authored-by: Codex GPT-6 <codex@openai.com>
Co-authored-by: Codex GPT-6 <codex@openai.com>
Propagate stratified IDs and bounds through select and filter conversion. Use inclusive stratification bounds for form date-distance operations and verify entity-date lower-bound intersection. Co-authored-by: Codex GPT-6 <codex@openai.com>
Co-authored-by: Codex GPT-6 <codex@openai.com>
| } | ||
|
|
||
| public ResolvedTableExportQuery map(TableExportQuery query, TranslatorDialect dialect) { | ||
| if (!(query.getQuery() instanceof ConceptQuery prerequisite)) { |
There was a problem hiding this comment.
das war bisher nicht so afaik, warum ändert sich das?
| concept.selects().stream().anyMatch(ResolvedConceptTranslator::isEventDateSelect), dialect, names); | ||
| Selects joinedSelects = joined.getQualifiedSelects(); | ||
| Map<String, String> connectorTables = new LinkedHashMap<>(); | ||
| connectors.forEach(item -> connectorTables.put(item.connector().table().physicalName().getLast(), |
There was a problem hiding this comment.
das würde zwei connectoren auf der selben tabelle überschreiben, ist vmtl kein besonders relevanter fall, sollten wir aber vermutlich im konzept abfedern
| import com.bakdata.conquery.sql.conversion.model.SqlQuery; | ||
|
|
||
| /** Backend boundary for resolved form compilation. */ | ||
| public final class ResolvedFormAdapter { |
There was a problem hiding this comment.
die queries werden hier nicht mehr validiert wie in ResolvedQueryAdapter
| /** A logical table together with its unquoted physical SQL name. */ | ||
| public record SqlTable( | ||
| @NotBlank String logicalId, | ||
| @NotEmpty List<@NotBlank String> physicalName |
There was a problem hiding this comment.
es gibt doch schon jooq name dafür?
| ) { | ||
|
|
||
| public AggregationConversionContext { | ||
| Objects.requireNonNull(dialect, "dialect"); |
| } | ||
|
|
||
| public static SingleColumnSqlSelect getSubstringSelect(ResolvedColumn column, Optional<SubstringRange> substringRange, String rootTable, String alias) { | ||
| Field<String> field = field(name(rootTable, column.physicalName()), String.class); |
There was a problem hiding this comment.
ich finde fast, dass das schon in ResolvedColumn sitzen könnte?
There was a problem hiding this comment.
das paket kann eigentlich weg, die Conditions sind eine ganz komische abstraktion die nicht gebraucht wird
| import org.jooq.impl.DSL; | ||
|
|
||
| /** A two-column date-range representation used by translate transformations. */ | ||
| public final class ColumnDateRange implements SqlSelect { |
awildturtok
left a comment
There was a problem hiding this comment.
Static follow-up focused on duplicate and parallel model types. I treated the resolved query/schema/operation DTOs as intentional module-boundary representations and commented only where the duplication looks avoidable or can drift.
| package com.bakdata.conquery.models.datasets; | ||
|
|
||
| /** Framework-neutral description of a validated dataset column. */ | ||
| public record ColumnDescriptor(String name, ColumnType type) { |
There was a problem hiding this comment.
This record is currently unreferenced in the changeset, while the backend already exposes models.query.ColumnDescriptor with different presentation semantics. The identical simple name is likely to cause confusion without providing value yet. Could we remove this until it has a consumer, or rename it to make the schema role explicit, e.g. DatasetColumnDescriptor?
|
|
||
| public static ResolvedColumn from(Column column) { | ||
| return new ResolvedColumn(column.getId().toString(), from(column.getTable()), column.getName(), | ||
| ColumnType.valueOf(column.getType().name()), true); |
There was a problem hiding this comment.
ColumnType duplicates all eight enum members of backend MajorTypeId, and valueOf(name()) makes their compatibility implicit: changing only one enum still compiles and then fails here at runtime. Could ColumnType be the canonical shared vocabulary, with MajorTypeId explicitly referencing it? At minimum, an exhaustive switch adapter would turn drift into a compile-time error.
|
|
||
| public ResolvedFormQuery map(RelativeFormQuery form) { | ||
| RelativeFormSettings settings = new RelativeFormSettings( | ||
| FormIndexSelector.valueOf(form.getIndexSelector().name()), |
There was a problem hiding this comment.
This exposes a parallel form vocabulary: FormIndexSelector, FormIndexPlacement, FormCalendarUnit, FormResolution, and FormAlignment duplicate backend enums and are all converted by name. A one-sided enum change will compile and fail only at runtime. Could the neutral enum identities live in dataset-model, with backend behavior operating on those shared values? Otherwise, exhaustive mappings would at least make drift visible during compilation.
| import jakarta.validation.constraints.NotNull; | ||
|
|
||
| /** Inclusive date restriction. An empty bound represents an unbounded side. */ | ||
| public record DateRange( |
There was a problem hiding this comment.
DateRange repeats the inclusive-bound and ordering model already represented by backend CDateRange/Range, while the neighboring NumberRange implements the new shared InclusiveRange. Could DateRange implement that shared abstraction as well (or use a common value object) so the extraction does not retain two separate inclusive-range APIs and validation implementations?
| private final SqlDateAggregator dateAggregator; | ||
| private final TranslatorDialect translatorDialect; | ||
|
|
||
| public ClickhouseDialectBundle() { |
| */ | ||
| public final class EntitySchemaAdapter { | ||
|
|
||
| private EntitySchemaAdapter() { |
| /** | ||
| * Converts legacy backend schema objects into the connector's resolved schema model. | ||
| * | ||
| * <p>TODO Remove this adapter once backend query resolution produces resolved connector schema objects directly.</p> |
| private final ResolvedFormMapper mapper; | ||
| private final FormSqlTranslator translator; | ||
|
|
||
| public ResolvedFormAdapter(ResolvedFormMapper mapper, FormSqlTranslator translator) { |
| /** Resolves initialized backend form DTOs into the connector-owned form model. */ | ||
| public final class ResolvedFormMapper { | ||
|
|
||
| private final ResolvedQueryMapper queryMapper; |
| private final ResolvedTableExportMapper mapper; | ||
| private final TableExportSqlTranslator translator; | ||
|
|
||
| public ResolvedTableExportAdapter(ResolvedTableExportMapper mapper, TableExportSqlTranslator compiler) { |
| List<Optional<ResolvedColumn>> output = new ArrayList<>(java.util.Collections.nCopies(width, Optional.empty())); | ||
| for (Column column : connector.getResolvedTable().getColumns()) { | ||
| Integer position = positions.get(column.getId()); | ||
| if (position != null) { |
| } | ||
| } | ||
| return new ResolvedExportTable( | ||
| concept.userLabel(Locale.ROOT), |
There was a problem hiding this comment.
Locale nicht abgeleitet?
| } | ||
|
|
||
| @Test | ||
| void shouldRouteAllFormTypesThroughTheResolvedTranslator() { |
| import org.jooq.impl.DSL; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| class SqlExtractionDialectTest { |
There was a problem hiding this comment.
=> bitte kein var verwenden
|
|
||
| /** Framework-neutral select operations supported by the SQL connector. */ | ||
| @UtilityClass | ||
| public final class BuiltInSelects { |
There was a problem hiding this comment.
Ich bin nicht happy, dass wir jetzt DSL(FE) => DSL(Resolved) => DSL(was die bakdata gemacht hat) => DSL(Jooq) machen.
awildturtok
left a comment
There was a problem hiding this comment.
Collection-copy review: one multiplicative allocation and the defensive-copy families found in the extracted SQL module.
| List<Param<?>> combined = new ArrayList<>(prefix.size() + 1); | ||
| combined.addAll(prefix); | ||
| combined.add(value); | ||
| next.add(List.copyOf(combined)); |
There was a problem hiding this comment.
[P2] Avoid copying every Cartesian-product tuplenncombined is freshly allocated and never mutated after insertion, so List.copyOf(combined) immediately duplicates every generated tuple. Since tuple count grows with the Cartesian product, add combined directly.
| public final class ModelNormalization { | ||
|
|
||
| public static <T> List<T> immutableCopy(List<T> values) { | ||
| return values == null ? null : Collections.unmodifiableList(new ArrayList<>(values)); |
There was a problem hiding this comment.
[P3] Remove model-wide defensive copyingnnThis helper exists solely to copy and wrap incoming collections. It is invoked by compact constructors across query nodes, operations, schemas, and translated results, frequently after callers already materialized a collection with toList() or List.of(). Store the supplied collections directly and remove ModelNormalization and those compact constructors.
| if (values == null) { | ||
| return null; | ||
| } | ||
| Map<String, List<String>> copy = new LinkedHashMap<>(); |
There was a problem hiding this comment.
[P3] Drop the deep defensive copynnConstruction copies the outer values map and every nested list, although ResolvedQueryMapper has just built the map for this entity and does not mutate it afterward. Store values directly; the validity-date copy is covered by the model-wide defensive-copy finding.
| List<ResolvedFilter> filters | ||
| ) { | ||
| public ResolvedExportTable { | ||
| outputColumns = List.copyOf(outputColumns); |
There was a problem hiding this comment.
[P3] Remove remaining form/export model copiesnnThese defensive copies duplicate lists already materialized by the backend mapper. The same pattern occurs in ResolvedTableExportQuery, ResolvedFormQuery, ResolvedFormMode, and RelativeFormSettings; remove all five compact constructors.
| List<MappingExpression> expressions | ||
| ) { | ||
| List<Field<?>> keyFields = collectKeyFields(expressions); | ||
| List<MappingExpression> resolvedExpressions = List.copyOf(expressions); |
There was a problem hiding this comment.
[P3] Remove mapping-layer defensive copiesnnThe resolver defensively freezes expressions, returned maps/lists, condition parameters, and expression maps. Related copies also occur in ConceptIdMappingSelection, ConceptIdMappingSource, and ConceptIdMappingTable. These objects can retain or return their existing collections directly; where only the collection type differs, accept Collection instead of materializing a new List.
| } | ||
|
|
||
| public SelectConversionContext { | ||
| conceptColumnTables = Map.copyOf(conceptColumnTables); |
There was a problem hiding this comment.
[P3] Remove translation-DTO defensive copiesnnThese map/list copies are repeated for every select conversion. The same pattern appears in ConceptColumnSource, ConceptCteInput, ConnectorCtePipelineInput, JoinBranchesCteInput, and PreprocessingCteInput; their producers already own the collections, so retain the references directly.
| /** | ||
| * All {@link QueryStep}'s that shall be converted before this {@link QueryStep}. | ||
| */ | ||
| /** All {@link QueryStep}s that must be converted before this step. */ | ||
| @Singular |
There was a problem hiding this comment.
[P3] Avoid generated collection copiesnnLombok's @Singular collection builder support copies plural inputs into builder storage and copies again when building; toBuilder() amplifies this. This affects QueryStep, Selects, WhereClauses, CommonAggregationSelect, ConceptSqlSelects, and ConnectorSqlSelects. Use ordinary builder fields with explicit empty defaults if collection immutability is not required.
| private final List<? extends Converter<? extends C, R, X>> converters; | ||
|
|
||
| public ConversionDispatcher(List<? extends Converter<? extends C, R, X>> converters) { | ||
| this.converters = List.copyOf(converters); |
There was a problem hiding this comment.
[P3] Retain the converter registrynnThis defensive copy is unnecessary for the fixed converter lists assembled by the callers. Store the supplied list directly.
| public SqlQuery overwriteResultInfos(List<ResultInfo> resultInfos) { | ||
| return new SqlQuery(sql, resultInfos); | ||
| public static SqlQuery fromCompiled(TranslatedQuery translatedQuery, List<ResultInfo> resultInfos) { | ||
| return new SqlQuery(translatedQuery.sql(), List.copyOf(resultInfos)); |
There was a problem hiding this comment.
[P3] Retain the result-info listnnresultInfos is already owned by the query conversion path; copying it here adds an allocation without protecting any subsequent mutation. Pass the list through directly.
No description provided.