search

risingwavelabs / dbt-risingwave

Apache License 2.0
21 stars 6 forks source link

refactor: on_configuration_change default to continue #36

Closed MattiasMTS closed 9 months ago

MattiasMTS commented 9 months ago

Suggestion to have the default values for index creation to be continue rather than apply. Docs: https://docs.getdbt.com/reference/resource-configs/on_configuration_change

According to the docs, if the on_configuration_change is not provided -> it defaults to apply. Perhaps this works well for other adapters, e.g. redshift, postgres, etc, for how their materialized view works but to me it feels like this is something we don't want for Risingwave.

Scenario: I've already built a model for the first time using 

dbt run -s <my-model>

If I run this command, again after creating it the first time, the model creation is "skipped" (because already exist) but the indexes creation are not skipped (if indexes are specified). I would expect the indexes to be skipped as well in this case if the on_configuration_change is by default empty.

chenzl25 commented 9 months ago

Good point! I also think the continue default behavior is more suitable for RisingWave.

MattiasMTS commented 9 months ago

Good point! I also think the continue default behavior is more suitable for RisingWave.

Let me know where we can highlight this in docs in the most suitable way :)

chenzl25 commented 9 months ago

Let me know where we can highlight this in docs in the most suitable way :)

Currently, the on_configuration_change default behavior hasn't been documented, but I will add it to the next release note.

MattiasMTS commented 9 months ago

I don't have "write access" in order to merge this PR @chenzl25. Feel free to do as seem fit my dude