Skip to content
This repository has been archived by the owner on Sep 12, 2024. It is now read-only.

fix(datasource): use case-insensitive check for LIMIT #1116

Merged
merged 1 commit into from
Sep 8, 2022

Conversation

Samyak2
Copy link
Contributor

@Samyak2 Samyak2 commented Sep 7, 2022

Before applying our own LIMIT 1, we check if the query already has a LIMIT clause by a simple substring search. This had two issues:

  • It did not work when LIMIT was in uppercase
  • It could match "limit" in a column or table name

Changed this to use a case-insensitive regex search which also looks for whitespace around LIMIT. Since "limit" as a column or table name will need to enclosed in double quotes, this regex will not match them.

@Samyak2 Samyak2 added the ❗alerts Alert formatting, scheduling, etc. label Sep 7, 2022
@Samyak2 Samyak2 added this to the v0.10.2 milestone Sep 7, 2022
@Samyak2 Samyak2 requested a review from manassolanki September 7, 2022 15:05
@gitpod-io
Copy link

gitpod-io bot commented Sep 7, 2022

@Samyak2 Samyak2 added 🐛 bug Something isn't working 🛠️ backend labels Sep 7, 2022
@netlify
Copy link

netlify bot commented Sep 7, 2022

Deploy Preview for frontend-sb canceled.

Name Link
🔨 Latest commit 95e90c8
🔍 Latest deploy log https://app.netlify.com/sites/frontend-sb/deploys/6318b467ce5f0100097d9bb6

@Samyak2 Samyak2 force-pushed the fix/test-query-allow-limit branch from 87c5c0b to 156edfd Compare September 7, 2022 15:09
Before applying our own `LIMIT 1`, we check if the query already has a
`LIMIT` clause by a simple substring search. This had two issues:
- It did not work when `LIMIT` was in uppercase
- It could match "limit" in a column or table name

Changed this to use a case-insensitive regex search which also looks for
whitespace around `LIMIT`. Since "limit" as a column or table name will
need to enclosed in double quotes, this regex will not match them.
@Samyak2 Samyak2 force-pushed the fix/test-query-allow-limit branch from 156edfd to 95e90c8 Compare September 7, 2022 15:10
@manassolanki manassolanki merged commit f4832fd into develop Sep 8, 2022
@Samyak2 Samyak2 deleted the fix/test-query-allow-limit branch September 8, 2022 04:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
❗alerts Alert formatting, scheduling, etc. 🛠️ backend 🐛 bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants