Skip to content

Prepare for merger with node-postgres #162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Prepare for merger with node-postgres #162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d8bce31
Revert "Throw when oid is not an integer (Fix #128) (#129)"
aqeelat Apr 10, 2025
e8a5a09
Revert the changes that broke compatibility with pg and pg-native
aqeelat Apr 10, 2025
c8e060f
specify timezone for tests
aqeelat Apr 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,6 @@ function setTypeParser (oid, format, parseFn) {
parseFn = format
format = 'text'
}
if (!Number.isInteger(oid)) {
throw new TypeError('oid must be an integer: ' + oid)
Comment on lines -34 to -35
Copy link
Author

Choose a reason for hiding this comment

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

This change breaks the check specified in:
https://github.com/brianc/node-postgres/blob/50c06f9bc6ff2ca1e8d7b7268b9af54ce49d72c1/packages/pg/test/unit/client/throw-in-type-parser-tests.js#L11-L13

}
typeParsers[format][oid] = parseFn
};

Expand Down
24 changes: 20 additions & 4 deletions lib/binaryParsers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,27 @@ const parseFloat64 = function (value) {
return value.readDoubleBE(0)
}

const parseTimestampUTC = function (value) {
const parseDate = function (isUTC, value) {
const rawValue = 0x100000000 * value.readInt32BE(0) + value.readUInt32BE(4)

// discard usecs and shift from 2000 to 1970
const result = new Date(Math.round(rawValue / 1000) + 946684800000)
const result = new Date((rawValue / 1000) + 946684800000)

if (!isUTC) {
result.setTime(result.getTime() + result.getTimezoneOffset() * 60000)
}

// add microseconds to the date
result.usec = rawValue % 1000
result.getMicroSeconds = function () {
return this.usec
}
result.setMicroSeconds = function (value) {
this.usec = value
}
result.getUTCMicroSeconds = function () {
return this.usec
}
Comment on lines +24 to +40
Copy link
Author

Choose a reason for hiding this comment

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

This change makes the types return a null object for some reason. I don't know why.

https://github.com/brianc/node-postgres/blob/50c06f9bc6ff2ca1e8d7b7268b9af54ce49d72c1/packages/pg/test/integration/client/api-tests.js#L245


return result
}
Expand Down Expand Up @@ -110,8 +126,8 @@ const init = function (register) {
register(700, parseFloat32)
register(701, parseFloat64)
register(16, parseBool)
register(1114, parseTimestampUTC)
register(1184, parseTimestampUTC)
register(1114, parseDate.bind(null, false))
register(1184, parseDate.bind(null, true))
register(1000, parseArray)
register(1007, parseArray)
register(1016, parseArray)
Expand Down
27 changes: 6 additions & 21 deletions lib/textParsers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const array = require('postgres-array')
const parseTimestampTz = require('postgres-date')
const parseTimestamp = require('postgres-date')
const parseInterval = require('postgres-interval')
const parseByteA = require('postgres-bytea')
const range = require('postgres-range')
Expand Down Expand Up @@ -40,22 +40,10 @@ const parseStringArray = function (value) {
return array.parse(value, undefined)
}

const parseTimestamp = function (value) {
const utc = value.endsWith(' BC')
? value.slice(0, -3) + 'Z BC'
: value + 'Z'

return parseTimestampTz(utc)
}
Comment on lines -43 to -49
Copy link
Author

Choose a reason for hiding this comment

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

this function was very problematic because it appends Z to the end of the timestamp, which breaks all tz related tests. It should be brought back later though.

Because this function was removed, I removed all of the range functions related to it.


const parseTimestampArray = function (value) {
return array.parse(value, parseTimestamp)
}

const parseTimestampTzArray = function (value) {
return array.parse(value, parseTimestampTz)
}

const parseIntervalArray = function (value) {
return array.parse(value, parseInterval)
}
Expand Down Expand Up @@ -131,10 +119,6 @@ function parseTimestampRange (raw) {
return range.parse(raw, parseTimestamp)
}

function parseTimestampTzRange (raw) {
return range.parse(raw, parseTimestampTz)
}

const init = function (register) {
register(20, parseBigInteger) // int8
register(21, Number) // int2
Expand All @@ -143,8 +127,9 @@ const init = function (register) {
register(700, parseFloat) // float4/real
register(701, parseFloat) // float8/double
register(16, parseBool)
register(1082, parseTimestamp) // date
Copy link
Author

Choose a reason for hiding this comment

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

This change makes it so that we return a date object instead of a string

register(1114, parseTimestamp) // timestamp without time zone
register(1184, parseTimestampTz) // timestamp with time zone
register(1184, parseTimestamp) // timestamp with time zone
register(600, parsePoint) // point
register(651, parseStringArray) // cidr[]
register(718, parseCircle) // circle
Expand All @@ -165,8 +150,8 @@ const init = function (register) {
register(1040, parseStringArray) // macaddr[]
register(1041, parseStringArray) // inet[]
register(1115, parseTimestampArray) // timestamp without time zone[]
register(1182, parseStringArray) // date[]
register(1185, parseTimestampTzArray) // timestamp with time zone[]
register(1182, parseTimestampArray) // date[]
Copy link
Author

Choose a reason for hiding this comment

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

This is related to 1082. We need to use a date parser instead of a string parser.

register(1185, parseTimestampArray) // timestamp with time zone[]
register(1186, parseInterval)
register(1187, parseIntervalArray)
register(17, parseByteA)
Expand All @@ -178,7 +163,7 @@ const init = function (register) {
register(3906, parseNumRange) // numrange
register(3907, parseStringArray) // numrange[]
register(3908, parseTimestampRange) // tsrange
register(3910, parseTimestampTzRange) // tstzrange
register(3910, parseTimestampRange) // tstzrange
register(3912, range.parse) // daterange
register(3926, parseInt8Range) // int8range
register(2951, parseStringArray) // uuid[]
Expand Down
14 changes: 3 additions & 11 deletions test/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@

const test = require('tape')
const { getTypeParser, setTypeParser } = require('../')
const getTypeParser = require('../').getTypeParser
const types = require('./types')

process.env.TZ = 'Asia/Riyadh'

test('types', function (t) {
Object.keys(types).forEach(function (typeName) {
const type = types[typeName]
Expand Down Expand Up @@ -64,14 +66,4 @@ test('types', function (t) {
t.equal(correct, 300)
t.end()
})

t.test('setTypeParser should throw when oid is not an integer', function (t) {
t.throws(function () {
setTypeParser(null, function () {})
}, /^TypeError: oid must be an integer/)
t.throws(function () {
setTypeParser('a', function () {})
}, /^TypeError: oid must be an integer/)
t.end()
})
})
59 changes: 31 additions & 28 deletions test/types.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,6 @@ exports.timestamptz = {
[
'2010-10-30 13:10:01+05',
dateEquals(2010, 9, 30, 8, 10, 1, 0)
],
[
'1000-01-01 00:00:00+00 BC',
dateEquals(-999, 0, 1, 0, 0, 0, 0)
Comment on lines -112 to -115
Copy link
Author

Choose a reason for hiding this comment

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

This was added to validated converting the datetime string to "Z BC" and is no longer needed.

]
]
}
Expand All @@ -125,17 +121,8 @@ exports.timestamp = {
'2010-10-31 00:00:00',
function (t, value) {
t.equal(
value.toISOString(),
'2010-10-31T00:00:00.000Z'
)
}
],
[
'1000-01-01 00:00:00 BC',
function (t, value) {
t.equal(
value.toISOString(),
'-000999-01-01T00:00:00.000Z'
value.toUTCString(),
new Date(2010, 9, 31, 0, 0, 0, 0).toUTCString()
Comment on lines -128 to +125
Copy link
Author

Choose a reason for hiding this comment

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

This was added to validated converting the datetime string to "Z BC" and is no longer needed. I reverted it to how it was before the change

)
}
]
Expand All @@ -146,8 +133,15 @@ exports.date = {
format: 'text',
id: 1082,
tests: [
['2010-10-31', '2010-10-31'],
['2010-10-31 BC', '2010-10-31 BC']
['2010-10-31', function (t, value) {
const now = new Date(2010, 9, 31)
dateEquals(
2010,
now.getUTCMonth(),
now.getUTCDate(),
now.getUTCHours(), 0, 0, 0)(t, value)
t.equal(value.getHours(), now.getHours())
}]
Comment on lines +136 to +144
Copy link
Author

Choose a reason for hiding this comment

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

This was added to validated converting the datetime string to "Z BC" and is no longer needed.

]
}

Expand Down Expand Up @@ -322,16 +316,16 @@ exports.tsrange = {
format: 'text',
id: 3908,
tests: [
['(2010-10-31 14:54:13.74,)', tsrangeEquals([[2010, 9, 31, 14, 54, 13, 74], null])],
['(2010-10-31 14:54:13.74,infinity)', tsrangeEquals([[2010, 9, 31, 14, 54, 13, 74], null])],
['(,2010-10-31 14:54:13.74)', tsrangeEquals([null, [2010, 9, 31, 14, 54, 13, 74]])],
['(-infinity,2010-10-31 14:54:13.74)', tsrangeEquals([null, [2010, 9, 31, 14, 54, 13, 74]])],
['(2010-10-30 10:54:13.74,2010-10-31 14:54:13.74)', tsrangeEquals([[2010, 9, 30, 10, 54, 13, 74], [2010, 9, 31, 14, 54, 13, 74]])],
['("2010-10-31 14:54:13.74",)', tsrangeEquals([[2010, 9, 31, 14, 54, 13, 74], null])],
['("2010-10-31 14:54:13.74",infinity)', tsrangeEquals([[2010, 9, 31, 14, 54, 13, 74], null])],
['(,"2010-10-31 14:54:13.74")', tsrangeEquals([null, [2010, 9, 31, 14, 54, 13, 74]])],
['(-infinity,"2010-10-31 14:54:13.74")', tsrangeEquals([null, [2010, 9, 31, 14, 54, 13, 74]])],
['("2010-10-30 10:54:13.74","2010-10-31 14:54:13.74")', tsrangeEquals([[2010, 9, 30, 10, 54, 13, 74], [2010, 9, 31, 14, 54, 13, 74]])]
['(2010-10-31 14:54:13.74,)', tsrangeEquals([[2010, 9, 31, 11, 54, 13, 74], null])],
['(2010-10-31 14:54:13.74,infinity)', tsrangeEquals([[2010, 9, 31, 11, 54, 13, 74], null])],
['(,2010-10-31 14:54:13.74)', tsrangeEquals([null, [2010, 9, 31, 11, 54, 13, 74]])],
['(-infinity,2010-10-31 14:54:13.74)', tsrangeEquals([null, [2010, 9, 31, 11, 54, 13, 74]])],
['(2010-10-30 10:54:13.74,2010-10-31 14:54:13.74)', tsrangeEquals([[2010, 9, 30, 7, 54, 13, 74], [2010, 9, 31, 11, 54, 13, 74]])],
['("2010-10-31 14:54:13.74",)', tsrangeEquals([[2010, 9, 31, 11, 54, 13, 74], null])],
['("2010-10-31 14:54:13.74",infinity)', tsrangeEquals([[2010, 9, 31, 11, 54, 13, 74], null])],
['(,"2010-10-31 14:54:13.74")', tsrangeEquals([null, [2010, 9, 31, 11, 54, 13, 74]])],
['(-infinity,"2010-10-31 14:54:13.74")', tsrangeEquals([null, [2010, 9, 31, 11, 54, 13, 74]])],
['("2010-10-30 10:54:13.74","2010-10-31 14:54:13.74")', tsrangeEquals([[2010, 9, 30, 7, 54, 13, 74], [2010, 9, 31, 11, 54, 13, 74]])]
Comment on lines +319 to +328
Copy link
Author

Choose a reason for hiding this comment

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

These were failing because of the tz mismatch between my machine and the runner. They should work now.

Also, there's a potential to use Date() objects here but I'm not familiar with the test harness to create fixtures.

]
}
exports.daterange = {
Expand Down Expand Up @@ -567,7 +561,16 @@ exports['array/date'] = {
id: 1182,
tests: [
['{2014-01-01,2015-12-31}', function (t, value) {
t.deepEqual(value, ['2014-01-01', '2015-12-31'])
const expecteds = [new Date(2014, 0, 1), new Date(2015, 11, 31)]
t.equal(value.length, 2)
value.forEach(function (date, index) {
const expected = expecteds[index]
dateEquals(
expected.getUTCFullYear(),
expected.getUTCMonth(),
expected.getUTCDate(),
expected.getUTCHours(), 0, 0, 0)(t, date)
})
}]
]
}
Expand Down