Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

bug: is_sorted should be true for SQLStream with replication keys #1950

Closed
pnadolny13 opened this issue Sep 11, 2023 · 2 comments · Fixed by #1951
Closed

bug: is_sorted should be true for SQLStream with replication keys #1950

pnadolny13 opened this issue Sep 11, 2023 · 2 comments · Fixed by #1951

Comments

@pnadolny13
Copy link
Contributor

Not sure if this is a bug or a design decision but shouldn't is_sorted be set to true in the default SQLStream implementation if we're adding an order by clause

query = query.order_by(replication_key_col)
?

Currently it accepts the default of False.

@edgarrmondragon what do you think?

@edgarrmondragon
Copy link
Collaborator

@pnadolny13 Yes, thanks for raising an actual issue 😅! I gave this a shot in tap-postgres a while ago (MeltanoLabs/tap-postgres#221) but doesn't seem to be as simple. We'd need to dive deeper.

@edgarrmondragon
Copy link
Collaborator

@pnadolny13 I have a PR for this: #1951. Let me know what you think.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants