Skip to content

feat: Add PostgreSQL support#8

Open
cB-Guru-Sharan-Kumar-Ram wants to merge 8 commits intoCodebucket-Solutions:masterfrom
cB-Guru-Sharan-Kumar-Ram:feature/postgresql-support
Open

feat: Add PostgreSQL support#8
cB-Guru-Sharan-Kumar-Ram wants to merge 8 commits intoCodebucket-Solutions:masterfrom
cB-Guru-Sharan-Kumar-Ram:feature/postgresql-support

Conversation

@cB-Guru-Sharan-Kumar-Ram
Copy link
Contributor

@cB-Guru-Sharan-Kumar-Ram cB-Guru-Sharan-Kumar-Ram commented Feb 11, 2026

Summary by CodeRabbit

  • New Features

    • Added configurable dialect support (defaults to MySQL) and a public identifier-quoting utility; quoting applied consistently across queries, aliases, joins, sorting and pagination.
  • Bug Fixes / Validation

    • Rejects PostgreSQL-unsupported JSON/set operators.
    • Stricter sort attribute normalization and validation; ORDER BY, filters, counts and table references are validated and quoted per dialect.

@coderabbitai
Copy link

coderabbitai bot commented Feb 11, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds dialect-aware identifier escaping and a public dialect property (default "mysql"). Introduces escapeIdentifier(identifier) and reads options.dialect in the constructor. Applies escaping across query assembly, adds camelCase sort-attribute validation, and throws for PostgreSQL-incompatible operators.

Changes

Cohort / File(s) Summary
Multi-dialect support & identifier escaping
index.js
Adds dialect (default "mysql") and public escapeIdentifier(); constructor reads options.dialect. Replaces direct identifier string concatenation with escapeIdentifier() across column rendering, SELECT/FROM/COUNT generation, ORDER BY, unions, single- and multi-table pagination, and filter field construction.
Sorting validation & operator guards
index.js
Introduces camelCase sort-attribute validation against option.columnList, builds dialect-aware ORDER BY parts using escapeIdentifier(), and enforces runtime errors for PostgreSQL when using JSON_CONTAINS, JSON_OVERLAPS, or FIND_IN_SET.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'feat: Add PostgreSQL support' directly and accurately summarizes the main objective of the changeset, which adds dialect support with specific focus on PostgreSQL compatibility.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
index.js (3)

234-234: ⚠️ Potential issue | 🟠 Major

Remove debug console.log statement.

console.log(replacements) will leak query parameters (potentially sensitive user data) to stdout in production.

Proposed fix
-    console.log(replacements);
     return {

313-316: ⚠️ Potential issue | 🟡 Minor

PostgreSQL requires quoted aliases for case-sensitive or reserved-word aliases in subqueries.

The subquery alias totalCounts on line 316 and countValue on line 203 are used unquoted. While these specific names happen to be safe, they are camelCase and PostgreSQL will fold them to lowercase. If any downstream code or the caller expects countValue (camelCase), it will receive countvalue instead. Consider quoting with escapeIdentifier or using snake_case aliases for PostgreSQL compatibility.


345-349: ⚠️ Potential issue | 🟠 Major

PostgreSQL queries with LIMIT will fail using the standard pg driver due to incompatible placeholder syntax.

The pg driver requires $1, $2, ... style placeholders, not ?. While the codebase has dialect detection for PostgreSQL (as shown by escapeIdentifier() and operator validation), the LIMIT clause at lines 345-349 hardcodes ? placeholders without checking the dialect. This means when dialect === "postgresql" is set, pagination queries will fail.

The fix requires conditional placeholder syntax based on the dialect (e.g., LIMIT $1,$2 for PostgreSQL vs. LIMIT ?,? for MySQL). Without this, PostgreSQL support is incomplete and non-functional for pagination.

🤖 Fix all issues with AI agents
In `@index.js`:
- Around line 330-335: The orderBy assembly is applying escapeIdentifier before
columnNameConverter; change it to call columnNameConverter first and then
escapeIdentifier so the converter receives the raw column name. Locate the code
that builds the orderBy string (the expression using orderByQuery,
this.columnNameConverter(this.escapeIdentifier(...)), sort.attributes[i], and
sort.sorts[i]) and swap the calls to use
this.escapeIdentifier(this.columnNameConverter(sort.attributes[i])) (i.e., call
columnNameConverter on the raw attribute, then pass that result into
escapeIdentifier) so the escaping follows the converter consistently with the
rest of the codebase.
- Around line 23-28: escapeIdentifier currently wraps raw input without escaping
delimiter characters, enabling SQL injection; update escapeIdentifier to
sanitize the identifier by (1) handling dot-qualified names by splitting on '.',
(2) for each segment replace any occurrences of the delimiter with the delimiter
doubled (for PostgreSQL double-quote -> "" , for MySQL backtick -> ``), (3) then
rejoin and wrap each escaped segment with the proper quoting character based on
this.dialect; also ensure it safely handles null/undefined and non-string inputs
(coerce to string) before escaping.
🧹 Nitpick comments (5)
index.js (5)

8-8: SqlString is imported but no longer used.

All usages of SqlString.escapeId have been replaced by this.escapeIdentifier. This is now a dead dependency import.

Proposed fix
-let SqlString = require('sqlstring');

158-175: Dead code: if (column) on line 161 is always true, and return [] on line 175 is unreachable.

Line 158-160 throws if !column, so execution only reaches line 161 when column is truthy. The guard on line 161 is redundant, and the return [] on line 175 can never be reached.

Proposed cleanup
          if (!column) {
            throw `Invalid filter field: ${field}`;
          }
-          if (column) {
-            let fieldName;
-    
-            if (column.statement) {
-              fieldName = column.statement;
-            } else if (column.prefix) {
-              fieldName = `${this.escapeIdentifier(column.prefix)}.${this.escapeIdentifier(column.name)}`;
-            } else {
-              fieldName = this.escapeIdentifier(column.name);
-            }
-    
-            return [[fieldName, operator, value]];
+          let fieldName;
+
+          if (column.statement) {
+            fieldName = column.statement;
+          } else if (column.prefix) {
+            fieldName = `${this.escapeIdentifier(column.prefix)}.${this.escapeIdentifier(column.name)}`;
+          } else {
+            fieldName = this.escapeIdentifier(column.name);
           }
-    
-          return []; // Return an empty array if no column matches
+
+          return [[fieldName, operator, value]];

33-37: Alias values are not escaped/quoted in AS clauses.

Column aliases (a.alias) are interpolated directly into the SQL string without any quoting or escaping. If an alias contains special characters or is a reserved word, the query will break or become vulnerable. Consider applying escapeIdentifier to aliases as well, for consistency with the rest of the identifier handling.

Also applies to: 43-44


54-56: Throwing string literals instead of Error objects.

The dialect-specific checks throw raw strings (e.g., throw "FIND_IN_SET is not supported..."). This loses the stack trace and makes error handling harder for callers (e.g., catch (e) won't have .stack). The same pattern exists elsewhere in the file (line 50, 159), but since these are new lines, consider using new Error(...).

Also applies to: 66-68


10-17: No validation of the dialect option.

The constructor silently accepts any string as dialect (e.g., "postgres", "pg", "mssql"). Invalid values would silently fall through to MySQL quoting in escapeIdentifier. Consider validating against a known set and throwing on unrecognized dialects.

Proposed validation
  constructor(options) {
+   const SUPPORTED_DIALECTS = ["mysql", "postgresql"];
    if(options) {
        let { columnNameConverter, dialect } = options;
        if (columnNameConverter) this.columnNameConverter = columnNameConverter;
-       this.dialect = dialect || "mysql";
+       this.dialect = dialect || "mysql";
+       if (!SUPPORTED_DIALECTS.includes(this.dialect)) {
+         throw new Error(`Unsupported dialect: "${this.dialect}". Supported: ${SUPPORTED_DIALECTS.join(", ")}`);
+       }
    } else {
        this.dialect = "mysql";
    }
  }

Also applies to: 20-21

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
index.js (3)

365-374: ⚠️ Potential issue | 🔴 Critical

Critical: LIMIT ?,? is MySQL-specific and will break on PostgreSQL.

Two issues here:

  1. Syntax: MySQL interprets LIMIT offset, count but PostgreSQL requires LIMIT <count> OFFSET <offset> — different keyword and reversed parameter order.
  2. Placeholders: PostgreSQL's pg driver uses $1, $2, … positional placeholders, not ?. If the consumer uses pg directly, every ? in the generated queries will fail.

Since this PR's purpose is to add PostgreSQL support, the query builder needs dialect-aware LIMIT/OFFSET and placeholder generation.

🐛 Proposed fix for LIMIT/OFFSET (placeholder translation is a broader concern)
    if (paginationObject.pageNo && paginationObject.itemsPerPage) {
      let offset =
        (paginationObject.pageNo - 1) * paginationObject.itemsPerPage;

-      query = query + " LIMIT ?,?";
-      replacements.push(offset, paginationObject.itemsPerPage);
+      if (this.dialect === "postgresql") {
+        query = query + " LIMIT ? OFFSET ?";
+        replacements.push(paginationObject.itemsPerPage, offset);
+      } else {
+        query = query + " LIMIT ?,?";
+        replacements.push(offset, paginationObject.itemsPerPage);
+      }
    } else if(paginationObject.offset && paginationObject.limit ) {
-      query = query + " LIMIT ?,?";
-      replacements.push(paginationObject.offset,paginationObject.limit)
+      if (this.dialect === "postgresql") {
+        query = query + " LIMIT ? OFFSET ?";
+        replacements.push(paginationObject.limit, paginationObject.offset);
+      } else {
+        query = query + " LIMIT ?,?";
+        replacements.push(paginationObject.offset, paginationObject.limit);
+      }
    }

Note: The ? placeholder issue affects all generated queries, not just LIMIT. Consider either documenting that consumers must use a ?-compatible driver (e.g., mysql2, knex), or adding dialect-aware placeholder indexing.


161-173: ⚠️ Potential issue | 🟠 Major

Bug: columnNameConverter is not applied to filter field names.

In columNames() (lines 37, 44), column names are passed through this.columnNameConverter before escaping. Here in the filter path, column.name is escaped directly without conversion. If a columnNameConverter is configured (e.g., camelCase → snake_case), the WHERE clause will reference unconverted column names that don't match the database schema.

🐛 Proposed fix
            if (column.statement) {
              fieldName = column.statement;
            } else if (column.prefix) {
-              fieldName = `${this.escapeIdentifier(column.prefix)}.${this.escapeIdentifier(column.name)}`;
+              fieldName = `${this.escapeIdentifier(column.prefix)}.${this.escapeIdentifier(this.columnNameConverter(column.name))}`;
            } else {
-              fieldName = this.escapeIdentifier(column.name);
+              fieldName = this.escapeIdentifier(this.columnNameConverter(column.name));
            }

234-234: ⚠️ Potential issue | 🟡 Minor

Remove debug console.log.

This leaks query replacement values (potentially sensitive data) to stdout in production.

🧹 Nitpick comments (3)
index.js (3)

328-347: toCamelCase is duplicated and redefined inside the loop.

The same toCamelCase helper is defined in both singleTablePagination (line 130) and here inside the for loop body (line 332), meaning it's recreated on every iteration. Extract it once as a class method or module-level utility.


54-68: Throw Error objects instead of string literals.

throw "FIND_IN_SET is not supported..." produces a plain string, which lacks a stack trace and won't be caught by error-handling middleware that checks instanceof Error. This pattern is used throughout the file (lines 50, 55, 67, 159, 325, 350). Consider using throw new Error(...) consistently.


277-281: Hardcoded "id" fallback sort assumes every table has an id column.

Line 279 unconditionally appends "id" to sort attributes, and line 349 special-cases it to bypass validation. PostgreSQL schemas commonly use different primary key names (e.g., <table>_id, UUIDs). Consider making the default tiebreaker sort column configurable per option/table.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (5)
index.js (5)

161-175: ⚠️ Potential issue | 🔴 Critical

Bug: columnNameConverter not applied to filter field names.

In the SELECT clause (columNames), column names go through this.columnNameConverter(a.name) before escaping. But here in processCondition, the filter field is built from column.name without applying columnNameConverter. When a converter is configured, the WHERE clause will reference different column names than the SELECT clause.

🐛 Proposed fix
            if (column.statement) {
              fieldName = column.statement;
            } else if (column.prefix) {
-              fieldName = `${this.escapeIdentifier(column.prefix)}.${this.escapeIdentifier(column.name)}`;
+              fieldName = `${this.escapeIdentifier(column.prefix)}.${this.escapeIdentifier(this.columnNameConverter(column.name))}`;
            } else {
-              fieldName = this.escapeIdentifier(column.name);
+              fieldName = this.escapeIdentifier(this.columnNameConverter(column.name));
            }

Also: the if (column) on line 161 is always true since line 158–159 throws when !column, making line 175 dead code.


234-234: ⚠️ Potential issue | 🟠 Major

Remove debug console.log.

This logs query replacement values (which may include user-supplied filter data or PII) to stdout. This should be removed before merging.


365-374: ⚠️ Potential issue | 🔴 Critical

Critical: LIMIT ?,? syntax is MySQL-only — PostgreSQL pagination is broken.

PostgreSQL does not support LIMIT offset, count. It requires LIMIT count OFFSET offset (note the reversed parameter order). Since this PR's purpose is to add PostgreSQL support, this is a showstopper.

🐛 Proposed fix
    if (paginationObject.pageNo && paginationObject.itemsPerPage) {
      let offset =
        (paginationObject.pageNo - 1) * paginationObject.itemsPerPage;

-      query = query + " LIMIT ?,?";
-      replacements.push(offset, paginationObject.itemsPerPage);
+      if (this.dialect === "postgresql") {
+        query = query + " LIMIT ? OFFSET ?";
+        replacements.push(paginationObject.itemsPerPage, offset);
+      } else {
+        query = query + " LIMIT ?,?";
+        replacements.push(offset, paginationObject.itemsPerPage);
+      }
    } else if(paginationObject.offset && paginationObject.limit ) {
-      query = query + " LIMIT ?,?";
-      replacements.push(paginationObject.offset,paginationObject.limit)
+      if (this.dialect === "postgresql") {
+        query = query + " LIMIT ? OFFSET ?";
+        replacements.push(paginationObject.limit, paginationObject.offset);
+      } else {
+        query = query + " LIMIT ?,?";
+        replacements.push(paginationObject.offset, paginationObject.limit);
+      }
    }

6-6: ⚠️ Potential issue | 🟠 Major

MySQL-specific operators RLIKE and MEMBER OF lack PostgreSQL guards.

JSON_CONTAINS, JSON_OVERLAPS, and FIND_IN_SET correctly throw for PostgreSQL, but RLIKE and MEMBER OF are also MySQL-specific and will produce invalid SQL on PostgreSQL. They need similar dialect guards in tupleCreator.

Also applies to: 47-90


365-381: ⚠️ Potential issue | 🟠 Major

PostgreSQL queries require $1, $2, ... placeholders, not ?.

The standard Node.js PostgreSQL driver (pg) uses numbered positional parameters like $1, $2, etc. The generated queries use ? placeholders (MySQL syntax), which will not work with the pg driver. If PostgreSQL support is a goal, either use a dialect-aware placeholder strategy or document this as a MySQL-only limitation.

🤖 Fix all issues with AI agents
In `@index.js`:
- Around line 202-206: The SQL alias for the COUNT column in totalCountQuery is
using mixed-case (countValue) which PostgreSQL will fold to lowercase; update
the query construction in totalCountQuery to quote the alias (e.g., AS
"countValue") so the returned column name preserves camelCase, and verify the
other COUNT alias usage (totalCounts) is consistent with consumer expectations
(either quote it or use a lowercase alias) to avoid undefined values when
reading the result.
- Around line 328-359: The validation currently uses toCamelCase to match
camelCase sort attributes against option.columnList entries but then builds
orderByQuery with the raw sortAttr; modify the loop that iterates
sort.attributes to store the matched column identifier (use col.name, col.alias,
or `${col.prefix}.${col.name}` as appropriate) into a variable like
resolvedSortAttr when isValidColumn becomes true, and after validation use this
resolvedSortAttr (passed through this.columnNameConverter and
this.escapeIdentifier) instead of the original sortAttr when appending to
orderByQuery; keep the existing check for "id" but ensure resolvedSortAttr falls
back to sortAttr only when no match was found.
- Around line 10-18: Validate the provided dialect in the constructor: check the
dialect option (used to set this.dialect) against an allowed set (e.g., "mysql",
"mariadb", "sqlite", "postgres"/"pg" etc. as your library supports) and if it is
not in that set throw a clear error instead of silently falling back to "mysql";
locate the constructor function and the references to options.dialect and
this.dialect and add a small validation branch that normalizes common aliases
(e.g., "pg" -> "postgres") or rejects unknown values with a descriptive
exception so users cannot unknowingly get MySQL quoting.
🧹 Nitpick comments (3)
index.js (3)

30-45: Aliases are not escaped with escapeIdentifier.

Column names are properly escaped via escapeIdentifier, but aliases (e.g., line 35, 40, 43) are interpolated raw into the query. While aliases are typically developer-controlled, applying escapeIdentifier to aliases would be consistent and defensive.

-            this.escapeIdentifier(a.prefix) + "." + this.escapeIdentifier(this.columnNameConverter(a.name)) + " AS " + a.alias
+            this.escapeIdentifier(a.prefix) + "." + this.escapeIdentifier(this.columnNameConverter(a.name)) + " AS " + this.escapeIdentifier(a.alias)

(Same pattern applies on lines 40, 43.)


54-68: Throwing string literals instead of Error objects.

Lines 55, 67 (and others like 50, 159, 325, 350) throw raw strings. This loses stack traces and breaks instanceof Error checks. Consider throwing proper Error instances.

-      throw `${operator} is not supported in PostgreSQL.`;
+      throw new Error(`${operator} is not supported in PostgreSQL.`);

130-136: Duplicated toCamelCase helper.

The same toCamelCase function is defined inline in both singleTablePagination (line 130) and paginate (line 332). Extract it as a class method or module-level utility.

Also applies to: 332-336

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
index.js (4)

234-234: ⚠️ Potential issue | 🟡 Minor

Remove debug console.log.

This leaks query replacements (which may contain sensitive user data) to stdout in production.

Proposed fix
-    console.log(replacements);

161-170: ⚠️ Potential issue | 🟠 Major

Bug: columnNameConverter is not applied to filter field names.

In columNames (line 37), column names pass through this.columnNameConverter(a.name) before escaping. But here in filter resolution, column.name is escaped directly without the converter. This inconsistency means filters reference the original DB column name while SELECT uses the converted name.

🐛 Proposed fix
            if (column.statement) {
              fieldName = column.statement;
            } else if (column.prefix) {
-              fieldName = `${this.escapeIdentifier(column.prefix)}.${this.escapeIdentifier(column.name)}`;
+              fieldName = `${this.escapeIdentifier(column.prefix)}.${this.escapeIdentifier(this.columnNameConverter(column.name))}`;
            } else {
-              fieldName = this.escapeIdentifier(column.name);
+              fieldName = this.escapeIdentifier(this.columnNameConverter(column.name));
            }

368-377: ⚠️ Potential issue | 🔴 Critical

Critical: LIMIT ?,? is MySQL-only syntax — breaks PostgreSQL.

MySQL's two-argument LIMIT offset, count form is not valid PostgreSQL. PostgreSQL requires LIMIT count OFFSET offset (note the reversed argument order too). This will cause a query syntax error for all PostgreSQL users attempting pagination.

🐛 Proposed fix
    if (paginationObject.pageNo && paginationObject.itemsPerPage) {
      let offset =
        (paginationObject.pageNo - 1) * paginationObject.itemsPerPage;

-      query = query + " LIMIT ?,?";
-      replacements.push(offset, paginationObject.itemsPerPage);
+      if (this.dialect === "postgresql") {
+        query = query + " LIMIT ? OFFSET ?";
+        replacements.push(paginationObject.itemsPerPage, offset);
+      } else {
+        query = query + " LIMIT ?,?";
+        replacements.push(offset, paginationObject.itemsPerPage);
+      }
    } else if(paginationObject.offset && paginationObject.limit ) {
-      query = query + " LIMIT ?,?";
-      replacements.push(paginationObject.offset,paginationObject.limit)
+      if (this.dialect === "postgresql") {
+        query = query + " LIMIT ? OFFSET ?";
+        replacements.push(paginationObject.limit, paginationObject.offset);
+      } else {
+        query = query + " LIMIT ?,?";
+        replacements.push(paginationObject.offset, paginationObject.limit);
+      }
    }

6-6: ⚠️ Potential issue | 🟠 Major

allowedOperators list includes MySQL-only operators without complete dialect guards.

MEMBER OF and RLIKE are MySQL-specific operators that are included in the global allowedOperators list but lack dialect guards in tupleCreator. Unlike JSON_CONTAINS, JSON_OVERLAPS, and FIND_IN_SET—which explicitly throw errors for PostgreSQL—MEMBER OF and RLIKE will silently produce invalid SQL when used with PostgreSQL. Add dialect checks similar to the existing guards for these operators.

🤖 Fix all issues with AI agents
In `@index.js`:
- Around line 346-349: The current assignment resolvedAttr = matchedCol.alias ||
matchedCol.name drops the prefix for columns discovered via the prefix.name
path; update the assignment inside the if (matchedCol) branch so that when a
prefix object is present you preserve it (e.g., use
`${prefix.name}.${matchedCol.alias || matchedCol.name}`) while still preferring
alias when available; modify the resolvedAttr assignment in the block that
handles matchedCol (the code using prefix.name and matchedCol) to build a
prefixed identifier rather than a bare column name.
- Around line 30-45: The columNames arrow function is inserting aliases
unescaped into SQL; update every branch that uses a.alias (including the prefix
branch, the statement branch, and the no-prefix branch) to pass the alias
through this.escapeIdentifier before interpolating (e.g., replace "AS " +
a.alias with "AS " + this.escapeIdentifier(a.alias)); keep existing uses of
this.columnNameConverter and escapeIdentifier for names and prefixes unchanged.
🧹 Nitpick comments (1)
index.js (1)

328-336: toCamelCase is duplicated — extract to a shared helper.

This same function is defined at line 130 inside singleTablePagination and again at line 332 inside paginate. Extract it once as a module-level utility or a class method.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
index.js (4)

234-234: ⚠️ Potential issue | 🟠 Major

Remove debug console.log — leaks query parameter values.

replacements can contain user-supplied data (search terms, filter values). Logging them in production is a compliance/privacy risk and exposes potentially sensitive information.

-    console.log(replacements);

161-170: ⚠️ Potential issue | 🟠 Major

columnNameConverter is not applied to filter field names, inconsistent with columNames.

In columNames (line 37), column names are passed through this.columnNameConverter(a.name) before escaping. Here in filter resolution (lines 167, 169), column.name is escaped directly without the converter. If columnNameConverter performs any transformation (e.g., camelCase ↔ snake_case), the WHERE clause column references will mismatch the SELECT clause.

Proposed fix
            } else if (column.prefix) {
-              fieldName = `${this.escapeIdentifier(column.prefix)}.${this.escapeIdentifier(column.name)}`;
+              fieldName = `${this.escapeIdentifier(column.prefix)}.${this.escapeIdentifier(this.columnNameConverter(column.name))}`;
            } else {
-              fieldName = this.escapeIdentifier(column.name);
+              fieldName = this.escapeIdentifier(this.columnNameConverter(column.name));
            }

373-382: ⚠️ Potential issue | 🔴 Critical

Critical: LIMIT ?,? syntax and parameter order are MySQL-specific — broken on PostgreSQL.

MySQL uses LIMIT offset, count while PostgreSQL uses LIMIT count OFFSET offset (different keyword and reversed parameter order). Since this PR adds PostgreSQL support, these queries will produce incorrect pagination results on PostgreSQL.

Proposed fix
    if (paginationObject.pageNo && paginationObject.itemsPerPage) {
      let offset =
        (paginationObject.pageNo - 1) * paginationObject.itemsPerPage;

-      query = query + " LIMIT ?,?";
-      replacements.push(offset, paginationObject.itemsPerPage);
+      if (this.dialect === "postgresql") {
+        query = query + " LIMIT ? OFFSET ?";
+        replacements.push(paginationObject.itemsPerPage, offset);
+      } else {
+        query = query + " LIMIT ?,?";
+        replacements.push(offset, paginationObject.itemsPerPage);
+      }
    } else if(paginationObject.offset && paginationObject.limit ) {
-      query = query + " LIMIT ?,?";
-      replacements.push(paginationObject.offset,paginationObject.limit)
+      if (this.dialect === "postgresql") {
+        query = query + " LIMIT ? OFFSET ?";
+        replacements.push(paginationObject.limit, paginationObject.offset);
+      } else {
+        query = query + " LIMIT ?,?";
+        replacements.push(paginationObject.offset, paginationObject.limit);
+      }
    }

6-8: ⚠️ Potential issue | 🟠 Major

Add guards for RLIKE and MEMBER OF operators; remove unused SqlString import.

  1. RLIKE and MEMBER OF are MySQL-specific operators but lack the PostgreSQL check present for JSON_CONTAINS, JSON_OVERLAPS, and FIND_IN_SET. Add similar guards in tupleCreator to prevent runtime errors on PostgreSQL.

  2. The sqlstring import on line 8 is unused and should be removed.

🧹 Nitpick comments (1)
index.js (1)

332-336: toCamelCase is duplicated and needlessly recreated per loop iteration.

This helper is identical to the one at line 130. Extract it as a module-level function or a class method to avoid duplication and per-iteration allocation.

+// At module level (near the top of the file)
+const toCamelCase = (str) => {
+  return str.replace(/_([a-zA-Z0-9])/g, (_, char) => {
+    return /[a-zA-Z]/.test(char) ? char.toUpperCase() : char;
+  });
+};

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.

1 participant