feat(#184): add source column on data_record #185
Conversation
There was a problem hiding this comment.
Thanks, @1yuv, for these changes. I recommend ensuring the new source field (or fields) is properly tested to verify that the new logic works as expected in all cases. Feel free to reach out if you need guidance on implementing these updates. Looking forward to seeing the final version!
| @@ -22,6 +22,7 @@ SELECT | |||
| document_metadata.saved_timestamp, | |||
| to_timestamp((NULLIF(doc->>'reported_date'::text, ''::text)::bigint / 1000)::double precision) AS reported, | |||
| doc->>'form' as form, | |||
| doc#>>'{fields,inputs,source}' as source, | |||
There was a problem hiding this comment.
Does breaking this into separate columns (fields, inputs, source) make sense?
This can improve clarity in downstream usage, simplify debugging and testing, and align with potential future changes of individual transformation.
There was a problem hiding this comment.
I don't see any benefits of breaking this into the separate columns. Moreover, that way is less performant and I've opened an issue to make this changes in every other places.
There was a problem hiding this comment.
Your suggestion to use direct access with #>> is an excellent approach for improving performance. If downstream processes don't need to select specific fields, inputs, or source attributes, separating them may not be necessary.
However, if more granularity is required, we could balance performance and flexibility by separating the columns while using direct access instead of ->.
Please let me know what you think.
| @@ -23,6 +23,8 @@ models: | |||
| data_type: timestamp with time zone | |||
| - name: form | |||
| data_type: string | |||
| - name: source | |||
There was a problem hiding this comment.
Did you consider adding generic data test here? You can refer to this documentation.
There was a problem hiding this comment.
I will also recommend updating the data_record fixtures to include the new fields along with their expected results.
Additionally, it would be interesting to have a sqltest to verify that the values for the new fields are correctly populated and consistent between data_record and couchdb.
For these updates, you can refer to the Guide for testing dbt models or review the existing tests already implemented in this repository. Let me know if you need any assistance!
Description
Add source column on data_record table.
Closes #184
License
The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.