feat(postgres): move postgres out to it's own import path, BREAKING CHANGE#454
Open
MrSwitch wants to merge 10 commits into
Open
feat(postgres): move postgres out to it's own import path, BREAKING CHANGE#454MrSwitch wants to merge 10 commits into
MrSwitch wants to merge 10 commits into
Conversation
MrSwitch
force-pushed
the
postgres-endpoint
branch
from
6cedd89 to
6c4e572
Compare
MrSwitch
changed the title
MrSwitch
force-pushed
the
postgres-endpoint
branch
from
52f69cd to
d908c90
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Moves Postgres-specific (and MySQL 5.7-specific) behaviour out of src/index.js into dedicated entry points (dare/postgres and dare/mysql57), introducing subclass-style prototypes that override engine-specific SQL generation (JSON aggregation, identifier quoting, fulltext search, ON DUPLICATE/CONFLICT, etc.). The base Dare is simplified to MySQL 8 defaults, and tests are re-organised into per-engine folders. This addresses issue #455 by making engine differences extensible via prototype overrides rather than inline engine.startsWith(...) checks.
Changes:
- New
src/postgres.jsandsrc/mysql57.jsfactories that wrapnew Dare()andsetPrototypeOfto engine-specific prototypes overridingsql_json_array,sql_json_arrayagg,identifierWrapper,onDuplicateKeysUpdate,fulltextSearch,jsonFormatValue, etc. - Base
src/index.jsremoves inline Postgres/MySQL-5 branches, replaces them with overridable prototype methods, andsrc/format/reducer_conditions.js+src/utils/group_concat.js+src/get.jsnow delegate todareInstance.*instead of switching onengine. - Package exposes new
./postgresand./mysql57subpath exports; tests reorganised intotest/specs/postgres/andtest/specs/mysql57/, with integrationhelpers/api.jspicking the constructor based onDB_ENGINE.
Reviewed changes
Copilot reviewed 39 out of 40 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/index.js | Strips engine branches; adds sql_json_array, sql_json_arrayagg, jsonFormatValue, fulltextSearch, sql_keyword_like defaults |
| src/postgres.js | New PostgresDare subclass with Postgres-specific overrides |
| src/mysql57.js | New MySQL57Dare subclass — note prototype overrides are written to Dare.prototype |
| src/utils/group_concat.js | Delegates JSON array/aggregate generation to dareInstance |
| src/format/reducer_conditions.js | Uses dareInstance.fulltextSearch, sql_keyword_like, jsonFormatValue |
| src/get.js | Passes dareInstance to group_concat |
| package.json | Adds ./postgres and ./mysql57 exports; consolidates lint into test scripts |
| README.md | Documents new dare/postgres import path |
| test/specs/postgres/*.spec.js | New Postgres test suites split out from generic specs |
| test/specs/mysql57/*.spec.js | New MySQL 5.7 test suites split out from generic specs |
| test/specs/*.spec.js | Removes engine-specific blocks; large formatting/Prettier-style updates |
| test/integration/helpers/api.js | Dynamically selects the constructor from DB_ENGINE env var |
| test/integration/{json,getFieldKey,disparities,shortcut_map,get}.spec.js | Switch to defaultAPI() helper / engine default mysql:8.0 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+77
to
+95
| Dare.prototype.applyCTELimitFiltering = function () { | ||
| return false; | ||
| }; | ||
|
|
||
| /** | ||
| * JSON quote values | ||
| * @param {any} value - Value to quote | ||
| * @returns {any} Quoted value | ||
| */ | ||
| Dare.prototype.jsonFormatValue = function jsonFormatValue(value) { | ||
| if (Array.isArray(value)) { | ||
| // In MySQL 5.7, we need to quote array values for IN | ||
| return value.map(jsonFormatValue); | ||
| } | ||
| if (typeof value === 'string') { | ||
| return `"${value}"`; | ||
| } | ||
| return value; | ||
| }; |
Comment on lines
+63
to
+69
| let condition = `IF(${sql_alias}.${rowid} IS NOT NULL, ${expression}, NULL)`; | ||
|
|
||
| if (this.engine.startsWith('mysql:5.7')) { | ||
| // Overwrite condition for MySQL 5.7 | ||
| condition = `IF(${sql_alias}.${rowid} IS NOT NULL, ${expression}, NULL)`; | ||
| } | ||
|
|
|
|
||
| // Mock instance of Dare | ||
| beforeEach(() => { | ||
| dareInstance = new MySQL57Dare({options, engine: 'mysql:5.7.0'}); |
Comment on lines
+65
to
+69
| * @returns {string} SQL clause | ||
| */ | ||
| PostgresDare.prototype.onDuplicateKeysUpdate = function onDuplicateKeysUpdate( | ||
| keys = [], | ||
| existing = [] |
Comment on lines
+14
to
+18
| "types": "./types/src/index.d.ts" | ||
| }, | ||
| "./mysql57": { | ||
| "import": "./src/mysql57.js", | ||
| "types": "./types/src/index.d.ts" |
| "test:integration": "bash ./test/integration/run.sh", | ||
| "spec": "c8 node --test test/specs/**/*.spec.js", | ||
| "spec": "c8 node --test 'test/specs/**/*.spec.js'", | ||
| "lint": "eslint ./ && npx prettier --check . && npm run check-types", |
Comment on lines
11
to
+33
| @@ -30,21 +28,9 @@ export default function group_concat({ | |||
| } | |||
|
|
|||
| // Convert to JSON Array | |||
| if (semverCompare(engine.split(':').at(1), '5.7') < 0) { | |||
| expression = fields.map( | |||
| field => | |||
| `'"', REPLACE(REPLACE(${field.expression}, '\\\\', '\\\\\\\\'), '"', '\\\\"'), '"'` | |||
| ); | |||
| expression = `CONCAT_WS('', '[', ${expression.join(", ',', ")}, ']')`; | |||
| } else { | |||
| // JSON_ARRAY in postgres default to ABSENT ON NULL, so we need to add NULL ON NULL | |||
| const json_array_settings = engine.startsWith('postgres') | |||
| ? ' NULL ON NULL' | |||
| : ''; | |||
|
|
|||
| expression = fields.map(field => field.expression); | |||
| expression = `JSON_ARRAY(${expression.join(',')}${json_array_settings})`; | |||
| } | |||
| expression = dareInstance.sql_json_array( | |||
| fields.map(field => field.expression) | |||
| ); | |||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #455