From 69277aa0430d951d61c485d2cd228c3cd9d4a33c Mon Sep 17 00:00:00 2001 From: Andrey Sidorov Date: Thu, 6 Jul 2023 21:03:00 +1000 Subject: [PATCH] feat: improved inspection of columns (#2112) * feat: improved inspection of column definition objects * remove console.log * lint * lint * display type length modifiers for non-default column lengths * add a unit test for column definition custom inspect * fix unit test * fix tests * fix unit test --- lib/packets/column_definition.js | 150 +++++++++++++++++- .../connection/test-column-inspect.mjs | 102 ++++++++++++ .../test-binary-multiple-results.js | 4 +- .../connection/test-execute-nocolumndef.js | 24 +-- .../connection/test-multiple-results.js | 4 +- test/unit/packets/test-column-definition.js | 9 +- 6 files changed, 272 insertions(+), 21 deletions(-) create mode 100644 test/builtin-runner/integration/connection/test-column-inspect.mjs diff --git a/lib/packets/column_definition.js b/lib/packets/column_definition.js index 1e38bb55e8..35b32a115f 100644 --- a/lib/packets/column_definition.js +++ b/lib/packets/column_definition.js @@ -66,14 +66,162 @@ class ColumnDefinition { table: this.table, orgTable: this.orgTable, characterSet: this.characterSet, + encoding: this.encoding, columnLength: this.columnLength, - columnType: this.columnType, type: this.columnType, flags: this.flags, decimals: this.decimals }; } + [Symbol.for('nodejs.util.inspect.custom')](depth, inspectOptions, inspect) { + const Types = require('../constants/types.js'); + const typeNames = []; + for (const t in Types) { + typeNames[Types[t]] = t; + } + const fiedFlags = require('../constants/field_flags.js'); + const flagNames = []; + // TODO: respect inspectOptions.showHidden + //const inspectFlags = inspectOptions.showHidden ? this.flags : this.flags & ~fiedFlags.PRI_KEY; + const inspectFlags = this.flags; + for (const f in fiedFlags) { + if (inspectFlags & fiedFlags[f]) { + if (f === 'PRI_KEY') { + flagNames.push('PRIMARY KEY'); + } else if (f === 'NOT_NULL') { + flagNames.push('NOT NULL'); + } else if (f === 'BINARY') { + // ignore flag for now + } else if (f === 'MULTIPLE_KEY') { + // not sure if that should be part of inspection. + // in the schema usually this is part of index definition + // example: UNIQUE KEY `my_uniq_id` (`id_box_elements`,`id_router`) + // note that only first column has MULTIPLE_KEY flag set in this case + // so there is no good way of knowing that this is part of index just + // by looking at indifidual field flags + } else if (f === 'NO_DEFAULT_VALUE') { + // almost the same as NOT_NULL? + } else if (f === 'BLOB') { + // included in the type + } else if (f === 'UNSIGNED') { + // this should be first after type + } else if (f === 'TIMESTAMP') { + // timestamp flag is redundant for inspection - already included in type + } else if (f === 'ON_UPDATE_NOW') { + flagNames.push('ON UPDATE CURRENT_TIMESTAMP'); + } else { + flagNames.push(f); + } + } + } + + if (depth > 1) { + return inspect({ + ...this.inspect(), + typeName: typeNames[this.columnType], + flags: flagNames, + }); + } + + const isUnsigned = this.flags & fiedFlags.UNSIGNED; + + let typeName = typeNames[this.columnType]; + if (typeName === 'BLOB') { + // TODO: check for non-utf8mb4 encoding + if (this.columnLength === 4294967295) { + typeName = 'LONGTEXT'; + } else if (this.columnLength === 67108860) { + typeName = 'MEDIUMTEXT'; + } else if (this.columnLength === 262140) { + typeName = 'TEXT'; + } else if (this.columnLength === 1020) { // 255*4 + typeName = 'TINYTEXT'; + } else { + typeName = `BLOB(${this.columnLength})`; + } + } else if (typeName === 'VAR_STRING') { + // TODO: check for non-utf8mb4 encoding + typeName = `VARCHAR(${Math.ceil(this.columnLength/4)})`; + } else if (typeName === 'TINY') { + if ( + (this.columnLength === 3 && isUnsigned) || + (this.columnLength === 4 && !isUnsigned) ) { + typeName = 'TINYINT'; + } else { + typeName = `TINYINT(${this.columnLength})`; + } + } else if (typeName === 'LONGLONG') { + if (this.columnLength === 20) { + typeName = 'BIGINT'; + } else { + typeName = `BIGINT(${this.columnLength})`; + } + } else if (typeName === 'SHORT') { + if (isUnsigned && this.columnLength === 5) { + typeName = 'SMALLINT'; + } else if (!isUnsigned && this.columnLength === 6) { + typeName = 'SMALLINT'; + } else { + typeName = `SMALLINT(${this.columnLength})`; + } + + } else if (typeName === 'LONG') { + if (isUnsigned && this.columnLength === 10) { + typeName = 'INT'; + } else if (!isUnsigned && this.columnLength === 11) { + typeName = 'INT'; + } else { + typeName = `INT(${this.columnLength})`; + } + } else if (typeName === 'INT24') { + if (isUnsigned && this.columnLength === 8) { + typeName = 'MEDIUMINT'; + } else if (!isUnsigned && this.columnLength === 9) { + typeName = 'MEDIUMINT'; + } else { + typeName = `MEDIUMINT(${this.columnLength})`; + } + } else if (typeName === 'DOUBLE') { + // DOUBLE without modifiers is reported as DOUBLE(22, 31) + if (this.columnLength === 22 && this.decimals === 31) { + typeName = 'DOUBLE'; + } else { + typeName = `DOUBLE(${this.columnLength},${this.decimals})`; + } + } else if (typeName === 'FLOAT') { + // FLOAT without modifiers is reported as FLOAT(12, 31) + if (this.columnLength === 12 && this.decimals === 31) { + typeName = 'FLOAT'; + } else { + typeName = `FLOAT(${this.columnLength},${this.decimals})`; + } + } else if (typeName === 'NEWDECIMAL') { + if (this.columnLength === 11 && this.decimals === 0) { + typeName = 'DECIMAL'; + } else if (this.decimals === 0) { + // not sure why, but DECIMAL(13) is reported as DECIMAL(14, 0) + // and DECIMAL(13, 9) is reported as NEWDECIMAL(15, 9) + if (isUnsigned) { + typeName = `DECIMAL(${this.columnLength})`; + } else { + typeName = `DECIMAL(${this.columnLength - 1})`; + } + } else { + typeName = `DECIMAL(${this.columnLength - 2},${this.decimals})`; + } + } else { + typeName = `${typeNames[this.columnType]}(${this.columnLength})`; + } + + if (isUnsigned) { + typeName += ' UNSIGNED'; + } + + // TODO respect colors option + return `\`${this.name}\` ${[typeName, ...flagNames].join(' ')}`; + } + static toPacket(column, sequenceId) { let length = 17; // = 4 padding + 1 + 12 for the rest fields.forEach(field => { diff --git a/test/builtin-runner/integration/connection/test-column-inspect.mjs b/test/builtin-runner/integration/connection/test-column-inspect.mjs new file mode 100644 index 0000000000..493e6711d2 --- /dev/null +++ b/test/builtin-runner/integration/connection/test-column-inspect.mjs @@ -0,0 +1,102 @@ +import { describe, it, before, after } from 'node:test'; +import assert from 'node:assert'; +import util from 'node:util'; +import common from '../../../common.js'; + +describe( + 'custom inspect for column definition', + { timeout: 1000 }, + async () => { + let connection; + + before(async () => { + connection = await common.createConnection().promise(); + connection.query(`DROP TABLE IF EXISTS test_fields`); + }); + + after(async () => { + await connection.end(); + }); + + it('should map fields to a schema-like description when depth is > 1', async () => { + const schema = ` + id INT NOT NULL AUTO_INCREMENT, + weight INT(2) UNSIGNED ZEROFILL, + usignedInt INT UNSIGNED NOT NULL, + signedInt INT NOT NULL, + unsignedShort SMALLINT UNSIGNED NOT NULL, + signedShort SMALLINT NOT NULL, + tinyIntUnsigned TINYINT UNSIGNED NOT NULL, + tinyIntSigned TINYINT NOT NULL, + mediumIntUnsigned MEDIUMINT UNSIGNED NOT NULL, + mediumIntSigned MEDIUMINT NOT NULL, + bigIntSigned BIGINT NOT NULL, + bigIntUnsigned BIGINT UNSIGNED NOT NULL, + longText_ LONGTEXT NOT NULL, + mediumText_ MEDIUMTEXT NOT NULL, + text_ TEXT NOT NULL, + tinyText_ TINYTEXT NOT NULL, + varString_1000 VARCHAR(1000) NOT NULL, + decimalDefault DECIMAL, + decimal13_10 DECIMAL(13,10), + floatDefault FLOAT, + float11_7 FLOAT(11,7), + dummyLastFieldToAllowForTrailingComma INT, + `; + await connection.query( + `CREATE TEMPORARY TABLE test_fields (${schema} PRIMARY KEY (id))` + ); + const [_, columns] = await connection.query('select * from test_fields'); + const inspectResults = util.inspect(columns); + const schemaArray = schema + .split('\n') + .map((line) => line.trim()) + .filter((line) => line.length > 0) + .map((line) => { + const words = line.split(' '); + const name = `\`${words[0]}\``; + return [name, ...words.slice(1)].join(' '); + }); + + const normalizedInspectResults = inspectResults + .split('\n') + .slice(1, -2) // remove "[" and "]" lines and also last dummy field + .map((line) => line.trim()) + // remove primary key - it's not in the schema explicitly but worth having in inspect + .map(line => line.split('PRIMARY KEY ').join('')); + + for (let l = 0; l < normalizedInspectResults.length; l++) { + const inspectLine = normalizedInspectResults[l]; + const schemaLine = schemaArray[l]; + assert.equal(inspectLine, schemaLine); + } + }); + + it.only('should show detailed description when depth is < 1', async () => { + await connection.query(` + CREATE TEMPORARY TABLE test_fields2 ( + id INT, + decimal13_10 DECIMAL(13,10) UNSIGNED NOT NULL, + PRIMARY KEY (id) + ) + `); + const [_, columns] = await connection.query('select * from test_fields2'); + const inspectResults = util.inspect(columns[1]); + assert.deepEqual(inspectResults, util.inspect({ + catalog: 'def', + schema: 'test', + name: 'decimal13_10', + orgName: 'decimal13_10', + table: 'test_fields2', + orgTable: 'test_fields2', + characterSet: 63, + encoding: 'binary', + columnLength: 14, + type: 246, + flags: [ 'NOT NULL' ], + decimals: 10, + typeName: 'NEWDECIMAL' + })); + }); + } +); diff --git a/test/integration/connection/test-binary-multiple-results.js b/test/integration/connection/test-binary-multiple-results.js index e3c8f4ed6a..ba5b60e962 100644 --- a/test/integration/connection/test-binary-multiple-results.js +++ b/test/integration/connection/test-binary-multiple-results.js @@ -43,7 +43,7 @@ const fields1 = [ { catalog: 'def', characterSet: 63, - columnType: 8, + encoding: 'binary', type: 8, decimals: 0, flags: 129, @@ -58,7 +58,7 @@ const nr_fields = [ { catalog: 'def', characterSet: 63, - columnType: 3, + encoding: 'binary', type: 3, decimals: 0, flags: 0, diff --git a/test/integration/connection/test-execute-nocolumndef.js b/test/integration/connection/test-execute-nocolumndef.js index 596856da95..ea810fb454 100644 --- a/test/integration/connection/test-execute-nocolumndef.js +++ b/test/integration/connection/test-execute-nocolumndef.js @@ -58,7 +58,7 @@ const expectedFields = [ table: '', orgTable: '', characterSet: 63, - columnType: 8, + encoding: 'binary', type: 8, flags: 161, decimals: 0 @@ -71,7 +71,7 @@ const expectedFields = [ table: '', orgTable: '', characterSet: 224, - columnType: 253, + encoding: 'utf8', type: 253, flags: 1, decimals: 31 @@ -84,7 +84,7 @@ const expectedFields = [ table: '', orgTable: '', characterSet: 224, - columnType: 253, + encoding: 'utf8', type: 253, flags: 0, decimals: 31 @@ -97,7 +97,7 @@ const expectedFields = [ table: '', orgTable: '', characterSet: 224, - columnType: 250, + encoding: 'utf8', type: 250, flags: 0, decimals: 31 @@ -110,7 +110,7 @@ const expectedFields = [ table: '', orgTable: '', characterSet: 224, - columnType: 253, + encoding: 'utf8', type: 253, flags: 0, decimals: 31 @@ -123,7 +123,7 @@ const expectedFields = [ table: '', orgTable: '', characterSet: 224, - columnType: 253, + encoding: 'utf8', type: 253, flags: 0, decimals: 31 @@ -136,7 +136,7 @@ const expectedFields = [ table: '', orgTable: '', characterSet: 224, - columnType: 253, + encoding: 'utf8', type: 253, flags: 0, decimals: 31 @@ -149,7 +149,7 @@ const expectedFields = [ table: '', orgTable: '', characterSet: 224, - columnType: 253, + encoding: 'utf8', type: 253, flags: 0, decimals: 31 @@ -162,7 +162,7 @@ const expectedFields = [ table: '', orgTable: '', characterSet: 224, - columnType: 253, + encoding: 'utf8', type: 253, flags: 0, decimals: 31 @@ -175,7 +175,7 @@ const expectedFields = [ table: '', orgTable: '', characterSet: 63, - columnType: 8, + encoding: 'binary', type: 8, flags: 160, decimals: 0 @@ -188,7 +188,7 @@ const expectedFields = [ table: '', orgTable: '', characterSet: 63, - columnType: 5, + encoding: 'binary', type: 5, flags: 128, decimals: 2 @@ -201,7 +201,7 @@ const expectedFields = [ table: '', orgTable: '', characterSet: 224, - columnType: 253, + encoding: 'utf8', type: 253, flags: 1, decimals: 31 diff --git a/test/integration/connection/test-multiple-results.js b/test/integration/connection/test-multiple-results.js index 449a33ded2..735ad8b087 100644 --- a/test/integration/connection/test-multiple-results.js +++ b/test/integration/connection/test-multiple-results.js @@ -42,7 +42,7 @@ const fields1 = [ { catalog: 'def', characterSet: 63, - columnType: 8, + encoding: 'binary', type: 8, decimals: 0, flags: 129, @@ -57,7 +57,7 @@ const nr_fields = [ { catalog: 'def', characterSet: 63, - columnType: 3, + encoding: 'binary', type: 3, decimals: 0, flags: 0, diff --git a/test/unit/packets/test-column-definition.js b/test/unit/packets/test-column-definition.js index dfe19e4aa9..ec91f2636d 100644 --- a/test/unit/packets/test-column-definition.js +++ b/test/unit/packets/test-column-definition.js @@ -19,7 +19,6 @@ let packet = ColumnDefinition.toPacket( columnLength: 500, flags: 32896, columnType: 0x8, - type: 0x8, decimals: 1 }, sequenceId @@ -38,12 +37,10 @@ packet = ColumnDefinition.toPacket( orgName: 'on_погоди', table: 't_погоди', orgTable: 'ot_погоди', - characterSet: 0x21, columnLength: 500, flags: 32896, columnType: 0x8, - type: 0x8, decimals: 1 }, sequenceId @@ -67,6 +64,7 @@ const inputColDef = { flags: 0, columnType: 0xfd, type: 0xfd, + encoding: 'latin1', decimals: 0x1f } packet = ColumnDefinition.toPacket(inputColDef, sequenceId); @@ -77,6 +75,9 @@ assert.equal( packet.offset = 4; const colDef = new ColumnDefinition(packet, "utf8"); -const inspect = colDef.inspect(); +// inspect omits the "colulumnType" property because type is an alias for it +// but ColumnDefinition.toPacket reads type from "columnType" +// TODO: think how to make this more consistent +const inspect = { columnType: 253, ...colDef.inspect() }; assert.deepEqual(inspect, inputColDef); assert.equal(colDef.db, inputColDef.schema);