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

#1296 use parameterized SQL query builder #1319

Merged

Conversation

junaidzm13
Copy link
Contributor

@junaidzm13 junaidzm13 commented May 1, 2024

  • this would prevent SQL injection and will also be adding better SQL support for different types i.e. for instance before we had to manually convert values to strings (with or without quotes depending on the type) before using them in SQL query. Now, ignite would do that for us.

Probably covers the following issue as well: #1232

Copy link

netlify bot commented May 1, 2024

Deploy Preview for papaya-valkyrie-395400 canceled.

Name Link
🔨 Latest commit cf98d15
🔍 Latest deploy log https://app.netlify.com/sites/papaya-valkyrie-395400/deploys/66321f219b050c00086927b1

@junaidzm13 junaidzm13 force-pushed the 1296-use-parameterized-sql-builder-for-ignite branch from 4ee7c7a to 3f656ae Compare May 1, 2024 06:54
@junaidzm13 junaidzm13 requested review from naleeha and chrisjstevo May 1, 2024 06:59
@junaidzm13 junaidzm13 self-assigned this May 1, 2024
@junaidzm13 junaidzm13 force-pushed the 1296-use-parameterized-sql-builder-for-ignite branch from 3f656ae to 978bd83 Compare May 1, 2024 10:46
- this would prevent SQL injection while adding better support
  for different types i.e. for instance before we had to manually
  convert values to strings (with or without quotes depending on
  the type) before using them in SQL query. Now, ignite would do
  that for us.
@junaidzm13 junaidzm13 force-pushed the 1296-use-parameterized-sql-builder-for-ignite branch from 978bd83 to cf98d15 Compare May 1, 2024 10:53
Copy link
Contributor

@naleeha naleeha left a comment

Choose a reason for hiding this comment

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

looks really good. very readable and extendable

@heswell heswell merged commit 09d7122 into finos:main May 1, 2024
13 of 14 checks passed
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