Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion cli/api/commands/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,13 @@ export class Builder {

const tableMetadataByTarget = new Map<string, dataform.ITableMetadata>();

// Normalize warehouse state targets to match compiled targets for consistent lookups.
// This is necessary because targets from the warehouse may have a database field set
// (from BigQuery credentials) while compiled targets may not have a database field
// if defaultDatabase is not specified in workflow_settings.yaml.
this.warehouseState.tables.forEach(tableState => {
tableMetadataByTarget.set(targetStringifier.stringify(tableState.target), tableState);
const normalizedTarget = this.normalizeWarehouseTarget(tableState.target);
tableMetadataByTarget.set(targetStringifier.stringify(normalizedTarget), tableState);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest to fix a bit differently: here we can set target instead of metadataTarget in the metadata structure.

It seems to me a more robust and concise solution.

});

const actions: dataform.IExecutionAction[] = [].concat(
Expand Down Expand Up @@ -121,4 +126,22 @@ export class Builder {
actionDescriptor: action.actionDescriptor
});
}

private normalizeWarehouseTarget(warehouseTarget: dataform.ITarget): dataform.ITarget {
// If the warehouse target has a database field and the compiled graph doesn't have a
// defaultDatabase, we should remove the database field from the warehouse target
// to match the format of the compiled targets. This ensures consistent lookups
// when defaultDatabase is not specified in workflow_settings.yaml.
if (
warehouseTarget.database &&
!this.prunedGraph.projectConfig.defaultDatabase
) {
return dataform.Target.create({
schema: warehouseTarget.schema,
name: warehouseTarget.name
});
}
// Otherwise return the warehouse target as-is
return warehouseTarget;
}
}
35 changes: 35 additions & 0 deletions core/actions/incremental_table_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -882,4 +882,39 @@ defaultIcebergConfig:
});
});
});

test("incremental table without defaultProject in workflow_settings", () => {
const projectDir = tmpDirFixture.createNewTmpDir();
// Create workflow_settings without defaultProject (only defaultDataset and defaultLocation)
fs.writeFileSync(
path.join(projectDir, "workflow_settings.yaml"),
`defaultDataset: dataform
defaultLocation: europe-west2
`
);
fs.mkdirSync(path.join(projectDir, "definitions"));
fs.writeFileSync(
path.join(projectDir, "definitions/incremental_table_without_default_project.sqlx"),
`config {
type: "incremental",
name: "incremental_table_without_default_project"
}

select \${incremental()} as is_incremental`
);

const result = runMainInVm(coreExecutionRequestFromPath(projectDir));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that this test will validate the fix because the problem is not on the compilation stage. But anyway the more tests the better :)

You'd need to add an integration test here https://github.com/dataform-co/dataform/blob/main/cli/index_test.ts#L951 to validate fix correctness, but I'm ok to accept PR without it.


expect(result.compile.compiledGraph.graphErrors.compilationErrors).deep.equals([]);
const compiledTable = result.compile.compiledGraph.tables[0];
// Verify the table compiles without database field since defaultProject is not set.
// The normalizeWarehouseTarget ensures that targets without database
// are correctly matched with warehouse state during the build phase, allowing incremental
// appends to work even when defaultProject is not specified in workflow_settings.yaml.
expect(compiledTable.type).equals("incremental");
expect(compiledTable.enumType).equals(dataform.TableType.INCREMENTAL);
expect(compiledTable.target.schema).equals("dataform");
expect(compiledTable.target.name).equals("incremental_table_without_default_project");
expect(compiledTable.target.database).equals("");
});
});