Skip to content

Conversation

RTLS
Copy link
Contributor

@RTLS RTLS commented Jul 21, 2025

No description provided.

@dosubot dosubot bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Jul 21, 2025
@RTLS RTLS mentioned this pull request Jul 21, 2025
Copy link
Contributor Author

RTLS commented Jul 21, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@dosubot dosubot bot added the enhancement New feature or request label Jul 21, 2025
@RTLS RTLS force-pushed the 07-20-_postgres_sink branch from 3f61302 to fd00020 Compare July 21, 2025 19:27
@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels Jul 21, 2025
Copy link
Contributor

@davoclavo davoclavo left a comment

Choose a reason for hiding this comment

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

Awesome! Looking forward to try out the PG sink soon! I am aware this PR is on the works but I'm leaving a couple of notes in case they are useful

end

@impl SinkPipeline
def handle_batch(:default, messages, _batch_info, context) do
Copy link
Contributor

@davoclavo davoclavo Jul 21, 2025

Choose a reason for hiding this comment

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

Just a reminder to call the routing logic here in the pipeline

{:ok, _} <-
Postgrex.query(
conn,
"INSERT INTO #{sink.table_name} (action, record, changes, metadata) VALUES ($1, $2, $3, $4)",
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess we should figure out a way to escape the table_name to avoid sql injections?

Copy link
Contributor

@davoclavo davoclavo Jul 21, 2025

Choose a reason for hiding this comment

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

Oh I noticed you added a validation to not include whitespace, good idea

In case a more restrictive validation were useful, here is the official definition of an identifier from the PG docs:

SQL identifiers and key words must begin with a letter (a-z, but also letters with diacritical marks and non-Latin letters) or an underscore (_). Subsequent characters in an identifier or key word can be letters, underscores, digits (0-9), or dollar signs ($). Note that dollar signs are not allowed in identifiers according to the letter of the SQL standard, so their use might render applications less portable. The SQL standard will not define a key word that contains digits or starts or ends with an underscore, so identifiers of this form are safe against possible conflict with future extensions of the standard.

https://www.postgresql.org/docs/17/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS

@RTLS
Copy link
Contributor Author

RTLS commented Jul 21, 2025

Awesome! Looking forward to try out the PG sink soon! I am aware this PR is on the works but I'm leaving a couple of notes in case they are useful

Yes this PR is very early still! Should have been draft. But I appreciate the early feedback 🙏

@RTLS RTLS force-pushed the 07-20-_add-sink.md branch from b572346 to c9b03c3 Compare July 28, 2025 21:26
@RTLS RTLS force-pushed the 07-20-_postgres_sink branch from fd00020 to 1a0aada Compare July 28, 2025 21:26
@RTLS RTLS force-pushed the 07-20-_postgres_sink branch from 1a0aada to 9f31e33 Compare August 6, 2025 22:12
@RTLS RTLS force-pushed the 07-20-_add-sink.md branch from c9b03c3 to 343c3c6 Compare August 6, 2025 22:12
Base automatically changed from 07-20-_add-sink.md to main August 6, 2025 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request size:XXL This PR changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants