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

Add java rule EC24: Optimize Database SQL Queries (Clause LIMIT) #279

Merged
merged 7 commits into from
Mar 17, 2024

Conversation

dedece35
Copy link
Member

@dedece35 dedece35 commented Mar 13, 2024

update rule-specifications for issue #239

@dedece35 dedece35 self-assigned this Mar 13, 2024
Copy link
Member

@utarwyn utarwyn left a comment

Choose a reason for hiding this comment

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

Would it make more sense to implement the EC24 rule for java rather than create a new one? This rule is not in the ecoCode RULES.md file, only the one in the JavaScript repository, which isn't very clear, I admit...

@dedece35 dedece35 changed the title Add java rule EC80 : Optimize Database SQL Queries (Clause LIMIT) Add java rule EC24: Optimize Database SQL Queries (Clause LIMIT) Mar 17, 2024
@dedece35 dedece35 merged commit 1064c24 into main Mar 17, 2024
1 check passed
@dedece35 dedece35 deleted the ISSUE_239 branch March 17, 2024 23:27
@Djoums
Copy link
Contributor

Djoums commented Mar 18, 2024

The rule makes sense but I don't see how it's going to be implemented. There are many ways to write SQL queries, and that's not even considering ORMs.

@dedece35
Copy link
Member Author

dedece35 commented Mar 18, 2024

Hi @Djoums,
I agree with you that there are a lot of ways to write one SQL query, and this is very difficult sometimes to check how these queries are built and what is the final SQL query really executed.
But, for me, we can implement the easiest way, firstly : if we detect a SQL query built inside a string, thus we can check if "limit" keyword is used or not. I'm ok with you, this use case is the easiest and I don't know if it's often used or not, but, these use cases will be detected.
what do you think about it ?

if you want you can a look to PRs :

@Djoums
Copy link
Contributor

Djoums commented Mar 18, 2024

Actually I don't think you can do it, even for the most basic cases. What if the dev writes this : string test = "select * from table". Are you going to raise a warning ? You don't know if that query will be executed or not, and you don't know if it will be limited or not (maybe there's test += " limit 25" somewhere later). You're going to raise a lot of false positives.

The way I see it, a code analyzer cannot enforce this rule. This would be a job for the query engine in the DBMS, because it's the only component that knows exactly what's going to be executed and how.

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

Successfully merging this pull request may close these issues.

3 participants