Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Possible bug in mssql_to_s3_operator #4

Open
mariana-s-fernandes opened this issue Aug 26, 2021 · 0 comments
Open

Possible bug in mssql_to_s3_operator #4

mariana-s-fernandes opened this issue Aug 26, 2021 · 0 comments

Comments

@mariana-s-fernandes
Copy link

I was using your code mssql_to_s3_operator.py as a reference point for a similar plugin that I am building. I've noticed something strange on the lines 198-203. To get the minimum and maximum values of the primary key, you are splitting the query_filter to get only the first part.

For example, starting with:

query_filter = "WHERE created_at >= x AND created_at < y AND"

(the last AND added on line 186), you would get:

count_sql_min = "SELECT min(id) FROM table WHERE created_at >= x"
count_sql_max = "SELECT max(id) FROM table WHERE created_at >= x"

This does not make much sense because you would get a count value much bigger than it should be, so you would end up with many more iterations on your loop, ie, you would end up looping during conditions where the filter condition was such that you wouldn't be loading data from the DB anymore (and I also don't see any break for when this happens).
Shouldn't we keep the full query_filter so that:

count_sql_max = "SELECT max(id) FROM table WHERE created_at >= x AND created_at < y"

Maybe the code was only tested using only the start argument without passing the end and in that case it would be removing the last "AND" added in line 186, so that the code works on this conditions.

Let me know if this makes sense or if I am missing anything, maybe there is some assumption that I am not considering.

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

No branches or pull requests

1 participant