Skip to content

Commit e5b93e0

Browse files
authored
Improving duplicate columns handling by ColumnSet (#978)
* declarations added * extend implemented * TS update * fully implemented * adding assertions * update the package * remove nyc output * adding tests * remove coverage
1 parent b5d4fb3 commit e5b93e0

File tree

8 files changed

+65
-50
lines changed

8 files changed

+65
-50
lines changed

.github/workflows/ci.yml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,6 @@ jobs:
7777
run: npm install
7878
- name: Initialize Database
7979
run: npm run test:init
80-
- name: Test and Coverage
81-
run: npm run coverage
8280
concurrency:
8381
group: ${{ github.workflow }}-${{ github.ref }}
8482
cancel-in-progress: true

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
.idea/
22
node_modules/
33
coverage/
4+
.nyc_output/
45
API/
56
typescript/*.js
67
typescript/*.map

lib/helpers/column-set.js

Lines changed: 41 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,12 @@ const npm = {
5656
*
5757
* By default, only the object's own properties are enumerated for column names.
5858
*
59+
* @param {'error' | 'skip' | 'replace'} [options.duplicate = 'error']
60+
* Strategy for processing duplicate columns:
61+
* - `'error'` - throws {@link external:Error Error} = `Duplicate column name "name".` (Default)
62+
* - `'skip'` - skips repeated columns (only the first will be used)
63+
* - `'replace'` - replaces repeated columns (the last one will be used)
64+
*
5965
* @returns {helpers.ColumnSet}
6066
*
6167
* @see
@@ -166,17 +172,17 @@ const npm = {
166172
*/
167173
class ColumnSet extends InnerState {
168174

169-
constructor(columns, opt) {
175+
constructor(columns, options) {
170176
super();
171177

172178
if (!columns || typeof columns !== 'object') {
173179
throw new TypeError('Invalid parameter \'columns\' specified.');
174180
}
175181

176-
opt = assert(opt, ['table', 'inherit']);
182+
options = assert(options, ['table', 'inherit', 'duplicate']);
177183

178-
if (!npm.utils.isNull(opt.table)) {
179-
this.table = (opt.table instanceof TableName) ? opt.table : new TableName(opt.table);
184+
if (!npm.utils.isNull(options.table)) {
185+
this.table = (options.table instanceof TableName) ? options.table : new TableName(options.table);
180186
}
181187

182188
/**
@@ -190,6 +196,7 @@ class ColumnSet extends InnerState {
190196
* - **secondary:** to be automatically written into the console (for logging purposes).
191197
*/
192198

199+
this.columns = [];
193200

194201
/**
195202
* @name helpers.ColumnSet#columns
@@ -199,22 +206,28 @@ class ColumnSet extends InnerState {
199206
* Array of {@link helpers.Column Column} objects.
200207
*/
201208
if (Array.isArray(columns)) {
202-
const colNames = {};
203-
this.columns = columns.map(c => {
209+
const duplicate = options.duplicate || 'error';
210+
columns.forEach(c => {
204211
const col = (c instanceof Column) ? c : new Column(c);
205-
if (col.name in colNames) {
206-
throw new Error(`Duplicate column name "${col.name}".`);
212+
const idx = this.columns.findIndex(a => a.name === col.name);
213+
if (idx === -1) {
214+
this.columns.push(col);
215+
} else {
216+
if (duplicate === 'replace') {
217+
this.columns[idx] = col;
218+
return;
219+
}
220+
if (duplicate !== 'skip') {
221+
throw new Error(`Duplicate column name "${col.name}".`);
222+
}
207223
}
208-
colNames[col.name] = true;
209-
return col;
210224
});
211225
} else {
212226
if (columns instanceof Column) {
213227
this.columns = [columns];
214228
} else {
215-
this.columns = [];
216229
for (const name in columns) {
217-
if (opt.inherit || Object.prototype.hasOwnProperty.call(columns, name)) {
230+
if (options.inherit || Object.prototype.hasOwnProperty.call(columns, name)) {
218231
this.columns.push(new Column(name));
219232
}
220233
}
@@ -415,13 +428,18 @@ ColumnSet.prototype.assignColumns = function (options) {
415428
* @description
416429
* Creates a new {@link helpers.ColumnSet ColumnSet}, by joining the two sets of columns.
417430
*
418-
* If the two sets contain a column with the same `name` (case-sensitive), an error is thrown.
431+
* If the two sets contain a column with the same `name` (case-sensitive), an error is thrown (unless `opts.skip` is set to `true`).
419432
*
420433
* @param {helpers.Column|helpers.ColumnSet|array} columns
421434
* Columns to be appended, of the same type as parameter `columns` during {@link helpers.ColumnSet ColumnSet} construction, except:
422435
* - it can also be of type {@link helpers.ColumnSet ColumnSet}
423436
* - it cannot be a simple object (properties enumeration is not supported here)
424437
*
438+
* @param {object} [options]
439+
*
440+
* @param {boolean} [options.skip = false]
441+
* When set to `true`, it skips duplicate columns (by default, it throws an error).
442+
*
425443
* @returns {helpers.ColumnSet}
426444
* New {@link helpers.ColumnSet ColumnSet} object with the extended/concatenated list of columns.
427445
*
@@ -465,13 +483,12 @@ ColumnSet.prototype.assignColumns = function (options) {
465483
* // ]
466484
* // }
467485
*/
468-
ColumnSet.prototype.extend = function (columns) {
469-
let cs = columns;
470-
if (!(cs instanceof ColumnSet)) {
471-
cs = new ColumnSet(columns);
472-
}
473-
// Any duplicate column will throw Error = 'Duplicate column name "name".',
474-
return new ColumnSet(this.columns.concat(cs.columns), {table: this.table});
486+
ColumnSet.prototype.extend = function (columns, options) {
487+
options = assert(options, ['skip']);
488+
const duplicate = options.skip ? 'skip' : 'error';
489+
const cs = columns instanceof ColumnSet ? columns : new ColumnSet(columns, {duplicate});
490+
const joinedCols = this.columns.concat(...cs.columns);
491+
return new ColumnSet(joinedCols, {table: this.table, duplicate});
475492
};
476493

477494
/**
@@ -536,23 +553,10 @@ ColumnSet.prototype.extend = function (columns) {
536553
*
537554
*/
538555
ColumnSet.prototype.merge = function (columns) {
539-
let cs = columns;
540-
if (!(cs instanceof ColumnSet)) {
541-
cs = new ColumnSet(columns);
542-
}
543-
const colNames = {}, cols = [];
544-
this.columns.forEach((c, idx) => {
545-
cols.push(c);
546-
colNames[c.name] = idx;
547-
});
548-
cs.columns.forEach(c => {
549-
if (c.name in colNames) {
550-
cols[colNames[c.name]] = c;
551-
} else {
552-
cols.push(c);
553-
}
554-
});
555-
return new ColumnSet(cols, {table: this.table});
556+
const duplicate = 'replace';
557+
const cs = columns instanceof ColumnSet ? columns : new ColumnSet(columns, {duplicate});
558+
const joinedCols = this.columns.concat(...cs.columns);
559+
return new ColumnSet(joinedCols, {table: this.table, duplicate});
556560
};
557561

558562
/**

lib/helpers/column.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ npm.utils.addInspection(Column, function () {
364364
* Used by methods {@link helpers.update update} and {@link helpers.sets sets}, ignored by methods {@link helpers.insert insert} and
365365
* {@link helpers.values values}. It indicates that the column is reserved for a `WHERE` condition, not to be set or updated.
366366
*
367-
* It can be set from a string initialization, by adding `?` in front of the name.
367+
* It can be set from a string initialization by adding `?` in front of the name.
368368
*
369369
* @property {*} [def]
370370
* Default value for the property, to be used only when the source object doesn't have the property.
@@ -379,7 +379,7 @@ npm.utils.addInspection(Column, function () {
379379
* Used by methods {@link helpers.update update} (for a single object) and {@link helpers.sets sets}, ignored by methods
380380
* {@link helpers.insert insert} and {@link helpers.values values}.
381381
*
382-
* It is also ignored when conditional flag `cnd` is set.
382+
* It is also ignored when the conditional flag `cnd` is set.
383383
*
384384
*/
385385

lib/main.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ let originalClientConnect;
3434
* @module pg-promise
3535
*
3636
* @description
37-
* ## pg-promise v12.0
37+
* ## pg-promise v12.5
3838
* All documentation here is for the latest official release only.
3939
*
4040
* ### Initialization Options

package.json

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
{
22
"name": "pg-promise",
3-
"version": "12.4.0",
3+
"version": "12.5.0",
44
"description": "PostgreSQL interface for Node.js",
55
"main": "lib/index.js",
66
"typings": "typescript/pg-promise.d.ts",
77
"scripts": {
88
"spelling": "cspell --config=.cspell.json \"**/*.{md,ts,js}\" --no-progress",
9-
"coverage": "istanbul cover ./node_modules/jasmine-node/bin/jasmine-node --captureExceptions test",
9+
"coverage": "nyc --reporter=html --reporter=text ./node_modules/jasmine-node/bin/jasmine-node --captureExceptions test",
1010
"doc": "jsdoc -c ./jsdoc/jsdoc.js ./jsdoc/README.md -t ./jsdoc/templates/custom",
1111
"lint": "eslint ./lib ./test/*.js ./test/db",
1212
"lint:fix": "npm run lint -- --fix",
@@ -42,12 +42,12 @@
4242
},
4343
"dependencies": {
4444
"assert-options": "0.8.3",
45-
"pg": "8.17.2",
45+
"pg": "8.18.0",
4646
"pg-minify": "1.8.0",
4747
"spex": "4.1.0"
4848
},
4949
"peerDependencies": {
50-
"pg-query-stream": "4.11.2"
50+
"pg-query-stream": "4.12.0"
5151
},
5252
"devDependencies": {
5353
"@eslint/js": "9.39.2",
@@ -56,10 +56,10 @@
5656
"cspell": "9.6.2",
5757
"eslint": "9.39.2",
5858
"globals": "17.2.0",
59-
"istanbul": "0.4.5",
6059
"jasmine-node": "3.0.0",
6160
"jsdoc": "4.0.5",
6261
"JSONStream": "1.3.5",
62+
"nyc": "17.1.0",
6363
"typescript": "5.9.3"
6464
}
6565
}

test/help.spec.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,10 @@ describe('ColumnSet', () => {
517517
expect(cs.columns[0].name).toBe('val');
518518
expect(cs.columns[1].name).toBe('msg');
519519
});
520+
it('must not ignore same columns when "skip" is set', () => {
521+
const cs = new helpers.ColumnSet(['one', 'one'], {duplicate: 'skip'});
522+
expect(cs.columns.length).toBe(1);
523+
});
520524
});
521525

522526
describe('property \'names\'', () => {
@@ -677,6 +681,10 @@ describe('ColumnSet', () => {
677681
cs.extend(['two']);
678682
}).toThrow('Duplicate column name "two".');
679683
});
684+
it('must ignore duplicates when "skip" is set', () => {
685+
const cs = new helpers.ColumnSet(['one', 'two']);
686+
expect(cs.extend(['one', 'two'], {skip: true}).columns.length).toBe(2);
687+
});
680688
});
681689

682690
describe('method \'merge\'', () => {
@@ -708,7 +716,6 @@ describe('ColumnSet', () => {
708716
new helpers.ColumnSet(['one', 'one']);
709717
}).toThrow('Duplicate column name "one".');
710718
});
711-
712719
it('must throw on invalid options', () => {
713720
expect(() => {
714721
new helpers.ColumnSet({}, 123);

typescript/pg-promise.d.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ declare namespace pgPromise {
130130
interface IColumnSetOptions {
131131
table?: string | ITable | TableName
132132
inherit?: boolean
133+
duplicate?: 'error' | 'skip' | 'replace'
133134
}
134135

135136
interface ITable {
@@ -206,8 +207,12 @@ declare namespace pgPromise {
206207
skip?: string | string[] | ((c: Column<T>) => boolean)
207208
}): string
208209

209-
extend<K extends string>(columns: K[]): ColumnSet<T & Record<K, unknown>>
210-
extend<S>(columns: Column<S> | ColumnSet<S> | Array<IColumnConfig<S> | Column<S>>): ColumnSet<T & S>
210+
extend<K extends string>(columns: K[], opts?: {
211+
skip?: boolean
212+
}): ColumnSet<T & Record<K, unknown>>
213+
extend<S>(columns: Column<S> | ColumnSet<S> | Array<IColumnConfig<S> | Column<S>>, opts?: {
214+
skip?: boolean
215+
}): ColumnSet<T & S>
211216

212217
merge<K extends string>(columns: K[]): ColumnSet<T & Record<K, unknown>>
213218
merge<S>(columns: Column<S> | ColumnSet<S> | Array<IColumnConfig<S> | Column<S>>): ColumnSet<T & S>

0 commit comments

Comments
 (0)