Skip to content
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.

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

decode timestamp without timezone as local DateTime and decode timestamp with timezone respecting the timezone defined in the connection #342

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# Changelog

## 3.3.0

- timeZone option in ConnectionSettings is now a TimeZoneSettings type instead of String
- add more flexibility on how timestamp and timestaptz types are decoded by adding flags to the TimeZoneSettings
- opcional decode timestamp without timezone as local DateTime and decode timestamp with timezone respecting the timezone defined in the connection

## 3.2.1

- Added or fixed decoders support for `QueryMode.simple`:
Expand Down
6 changes: 5 additions & 1 deletion lib/postgres.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import 'dart:io';

import 'package:collection/collection.dart';
import 'package:meta/meta.dart';
import 'package:postgres/src/timezone_settings.dart';
import 'package:stream_channel/stream_channel.dart';

import 'src/replication.dart';
Expand All @@ -16,6 +17,9 @@ import 'src/v3/query_description.dart';
export 'src/exceptions.dart';
export 'src/pool/pool_api.dart';
export 'src/replication.dart';

export 'src/timezone_settings.dart';

export 'src/types.dart';
export 'src/types/geo_types.dart';
export 'src/types/range_types.dart';
Expand Down Expand Up @@ -440,7 +444,7 @@ enum SslMode {

class ConnectionSettings extends SessionSettings {
final String? applicationName;
final String? timeZone;
final TimeZoneSettings? timeZone;
final Encoding? encoding;
final SslMode? sslMode;

Expand Down
5 changes: 4 additions & 1 deletion lib/src/buffer.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import 'dart:convert';

import 'package:buffer/buffer.dart';
import 'package:postgres/src/timezone_settings.dart';

/// This class doesn't add much over using `List<int>` instead, however,
/// it creates a nice explicit type difference from both `String` and `List<int>`,
Expand Down Expand Up @@ -43,9 +44,11 @@ const _emptyString = '';

class PgByteDataReader extends ByteDataReader {
final Encoding encoding;

final TimeZoneSettings timeZone;

PgByteDataReader({
required this.encoding,
required this.timeZone,
});

String readNullTerminatedString() {
Expand Down
88 changes: 65 additions & 23 deletions lib/src/message_window.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import 'dart:typed_data';

import 'package:buffer/buffer.dart';
import 'package:charcode/ascii.dart';
import 'package:postgres/src/timezone_settings.dart';

import 'buffer.dart';
import 'messages/server_messages.dart';
Expand All @@ -14,32 +15,73 @@ const int _headerByteSize = 5;
typedef _ServerMessageFn = ServerMessage Function(
PgByteDataReader reader, int length);

Map<int, _ServerMessageFn> _messageTypeMap = {
49: (_, __) => ParseCompleteMessage(),
50: (_, __) => BindCompleteMessage(),
65: (r, _) => NotificationResponseMessage.parse(r),
67: (r, _) => CommandCompleteMessage.parse(r),
68: (r, _) => DataRowMessage.parse(r),
69: ErrorResponseMessage.parse,
75: (r, _) => BackendKeyMessage.parse(r),
82: AuthenticationMessage.parse,
83: (r, l) => ParameterStatusMessage.parse(r),
84: (r, _) => RowDescriptionMessage.parse(r),
87: (r, _) => CopyBothResponseMessage.parse(r),
90: ReadyForQueryMessage.parse,
100: _parseCopyDataMessage,
110: (_, __) => NoDataMessage(),
116: (r, _) => ParameterDescriptionMessage.parse(r),
$3: (_, __) => CloseCompleteMessage(),
$N: NoticeMessage.parse,
};
// Map<int, _ServerMessageFn> _messageTypeMap = {
Copy link
Owner

Choose a reason for hiding this comment

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

Is there a reason for changing this to a switch statement? If all else being equal, let's not change this (or if this is needed, let's do this in a different PR).

Copy link
Author

Choose a reason for hiding this comment

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

At first I thought I needed to change this but then I saw that it wouldn't be necessary, it turns out that I forgot to revert this change.

// 49: (_, __) => ParseCompleteMessage(),
// 50: (_, __) => BindCompleteMessage(),
// 65: (r, _) => NotificationResponseMessage.parse(r),
// 67: (r, _) => CommandCompleteMessage.parse(r),
// 68: (r, _) => DataRowMessage.parse(r),
// 69: ErrorResponseMessage.parse,
// 75: (r, _) => BackendKeyMessage.parse(r),
// 82: AuthenticationMessage.parse,
// 83: (r, l) => ParameterStatusMessage.parse(r),
// 84: (r, _) => RowDescriptionMessage.parse(r),
// 87: (r, _) => CopyBothResponseMessage.parse(r),
// 90: ReadyForQueryMessage.parse,
// 100: _parseCopyDataMessage,
// 110: (_, __) => NoDataMessage(),
// 116: (r, _) => ParameterDescriptionMessage.parse(r),
// $3: (_, __) => CloseCompleteMessage(),
// $N: NoticeMessage.parse,
// };

class MessageFramer {
final Encoding _encoding;
late final _reader = PgByteDataReader(encoding: _encoding);
TimeZoneSettings timeZone;
late final _reader = PgByteDataReader(encoding: _encoding, timeZone: timeZone);
final messageQueue = Queue<ServerMessage>();

MessageFramer(this._encoding);
MessageFramer(this._encoding, this.timeZone);

_ServerMessageFn? _messageTypeMap(int? messageType) {
switch (messageType) {
case 49:
return (_, __) => ParseCompleteMessage();
case 50:
return (_, __) => BindCompleteMessage();
case 65:
return (r, _) => NotificationResponseMessage.parse(r);
case 67:
return (r, _) => CommandCompleteMessage.parse(r);
case 68:
return (r, _) => DataRowMessage.parse(r);
case 69:
return ErrorResponseMessage.parse;
case 75:
return (r, _) => BackendKeyMessage.parse(r);
case 82:
return AuthenticationMessage.parse;
case 83:
return (r, l) => ParameterStatusMessage.parse(r);
case 84:
return (r, _) => RowDescriptionMessage.parse(r);
case 87:
return (r, _) => CopyBothResponseMessage.parse(r);
case 90:
return ReadyForQueryMessage.parse;
case 100:
return _parseCopyDataMessage;
case 110:
return (_, __) => NoDataMessage();
case 116:
return (r, _) => ParameterDescriptionMessage.parse(r);
case $3:
return (_, __) => CloseCompleteMessage();
case $N:
return NoticeMessage.parse;
}
return null;
}

int? _type;
int _expectedLength = 0;
Expand Down Expand Up @@ -69,7 +111,7 @@ class MessageFramer {
}

if (_hasReadHeader && _isComplete) {
final msgMaker = _messageTypeMap[_type];
final msgMaker = _messageTypeMap(_type);
if (msgMaker == null) {
_addMsg(UnknownMessage(_type!, _reader.read(_expectedLength)));
continue;
Expand Down Expand Up @@ -116,7 +158,7 @@ ServerMessage _parseCopyDataMessage(PgByteDataReader reader, int length) {
if (code == ReplicationMessageId.primaryKeepAlive) {
return PrimaryKeepAliveMessage.parse(reader);
} else if (code == ReplicationMessageId.xLogData) {
return XLogDataMessage.parse(reader.read(length - 1), reader.encoding);
return XLogDataMessage.parse(reader.read(length - 1), reader.encoding, reader.timeZone);
} else {
final bb = BytesBuffer();
bb.addByte(code);
Expand Down
5 changes: 3 additions & 2 deletions lib/src/messages/client_messages.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import 'dart:convert';
import 'dart:typed_data';

import 'package:charcode/ascii.dart';
import 'package:postgres/src/timezone_settings.dart';
import 'package:postgres/src/types/generic_type.dart';

import '../buffer.dart';
Expand Down Expand Up @@ -49,12 +50,12 @@ class StartupMessage extends ClientMessage {

StartupMessage({
required String database,
required String timeZone,
required TimeZoneSettings timeZone,
String? username,
String? applicationName,
ReplicationMode replication = ReplicationMode.none,
}) : _databaseName = database,
_timeZone = timeZone,
_timeZone = timeZone.value,
_username = username,
_applicationName = applicationName,
_replication = replication.value;
Expand Down
9 changes: 7 additions & 2 deletions lib/src/messages/server_messages.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import 'dart:convert';
import 'dart:typed_data';

import 'package:meta/meta.dart';
import 'package:postgres/src/timezone_settings.dart';


import '../buffer.dart';
import '../time_converters.dart';
Expand Down Expand Up @@ -69,6 +71,9 @@ class ParameterStatusMessage extends ServerMessage {
factory ParameterStatusMessage.parse(PgByteDataReader reader) {
final name = reader.readNullTerminatedString();
final value = reader.readNullTerminatedString();
if (name.toLowerCase() == 'timezone') {
reader.timeZone.value = value;
Copy link
Owner

Choose a reason for hiding this comment

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

Let's not update the timeZone value like this, it is a bit unexpected here. The v3/connection.dart _handleMessage already stores it in PgConnectionImplementation._parameters and we should just read that value from there. I think the settings should be immutable (maybe rename TimeZoneSettings.value into defaultTimeZone), and use the server-provided timeZone if present, otherwise the fallback.

Copy link
Author

Choose a reason for hiding this comment

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

I believe that in this case it should not be immutable because when you use the SQL command "set timezone TO 'GMT';" you are changing the connection configuration, and this has to be reflected in the connection instance, because if after the user executes the SQL command to change the timezone and he reads the timezone property he will want to see the current value, right? I don't know the driver code very well, especially version 3, so I don't know if the driver user can read the timezone of the current connection through some method or property, as I have been very busy I haven't had much time to look at this, I don't know exactly how this could be done in another way.

}
return ParameterStatusMessage._(name, value);
}
}
Expand Down Expand Up @@ -366,8 +371,8 @@ class XLogDataMessage implements ReplicationMessage, ServerMessage {
/// If [XLogDataMessage.data] is a [LogicalReplicationMessage], then the method
/// will return a [XLogDataLogicalMessage] with that message. Otherwise, it'll
/// return [XLogDataMessage] with raw data.
static XLogDataMessage parse(Uint8List bytes, Encoding encoding) {
final reader = PgByteDataReader(encoding: encoding)..add(bytes);
static XLogDataMessage parse(Uint8List bytes, Encoding encoding, TimeZoneSettings timeZone) {
final reader = PgByteDataReader(encoding: encoding, timeZone: timeZone)..add(bytes);
final walStart = LSN(reader.readUint64());
final walEnd = LSN(reader.readUint64());
final time = dateTimeFromMicrosecondsSinceY2k(reader.readUint64());
Expand Down
34 changes: 34 additions & 0 deletions lib/src/timezone_settings.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/// A class to configure time zone settings for decoding timestamps and dates.
class TimeZoneSettings {
/// The default time zone value.
///
/// The [value] represents the name of the time zone location. Default is 'UTC'.
String value = 'UTC';

/// Creates a new instance of [TimeZoneSettings].
///
/// [value] is the name of the time zone location.
///
/// The optional named parameters:
/// - [forceDecodeTimestamptzAsUTC]: if true, decodes timestamps with timezone (timestamptz) as UTC. If false, decodes them using the timezone defined in the connection.
/// - [forceDecodeTimestampAsUTC]: if true, decodes timestamps without timezone (timestamp) as UTC. If false, decodes them as local datetime.
/// - [forceDecodeDateAsUTC]: if true, decodes dates as UTC. If false, decodes them as local datetime.
TimeZoneSettings(
this.value, {
this.forceDecodeTimestamptzAsUTC = true,
this.forceDecodeTimestampAsUTC = true,
this.forceDecodeDateAsUTC = true,
});

/// If true, decodes the timestamp with timezone (timestamptz) as UTC.
/// If false, decodes the timestamp with timezone using the timezone defined in the connection.
bool forceDecodeTimestamptzAsUTC = true;

/// If true, decodes the timestamp without timezone (timestamp) as UTC.
/// If false, decodes the timestamp without timezone as local datetime.
bool forceDecodeTimestampAsUTC = true;

/// If true, decodes the date as UTC.
/// If false, decodes the date as local datetime.
bool forceDecodeDateAsUTC = true;
}
Loading