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

bug(pgwire): failed to fetch TIMESTAMPTZ data using psycopg3 #18079

Open
cloudcarver opened this issue Aug 18, 2024 · 3 comments
Open

bug(pgwire): failed to fetch TIMESTAMPTZ data using psycopg3 #18079

cloudcarver opened this issue Aug 18, 2024 · 3 comments
Assignees
Milestone

Comments

@cloudcarver
Copy link
Contributor

Reproduced in

  1. Linux binary distribution installed via script, version: risingwave 1.10.0 (0542faf)
  2. A cluster hosted in the cloud, version: v1.9.2.
  3. Docker iamge distribution, image tag: v1.10.0

My environment:

Linux mike 5.15.153.1-microsoft-standard-WSL2 #1 SMP Fri Mar 29 23:14:13 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Python 3.12.5

To Reproduce

  1. Install psycopg
pip install "psycopg[binary,pool]" 
  1. Run RisingWave
docker run --rm -p 4566:4566 risingwavelabs/risingwave:v1.10.0
  1. Run the following Python script
import psycopg
# conn = psycopg.connect("postgresql://postgres:postgres@localhost:5432/postgres")
conn = psycopg.connect("postgresql://root:@localhost:4566/dev")
with conn.cursor() as cursor:
  cursor.execute("SELECT '2024-08-18 14:35:00+00:00'::TIMESTAMPTZ")
conn.close()

Then, the Python process will crash:

[1]    104989 segmentation fault  python main.py

You can also try it with Postgres:

docker run --rm -p 5432:5432 -e POSTGRES_PASSWORD=postgres postgres:latest
# connection string: postgresql://postgres:postgres@localhost:5432/postgres

There is no problem when using Postgres. So it's not the problem of psycopg I guess?

@github-actions github-actions bot added this to the release-2.0 milestone Aug 18, 2024
@cloudcarver cloudcarver changed the title failed to fetch TIMESTAMPTZ data using psycopg failed to fetch TIMESTAMPTZ data using psycopg3 Aug 18, 2024
@cloudcarver
Copy link
Contributor Author

confirmed that psycopg2 has no such issue

Copy link
Contributor

This issue has been open for 60 days with no activity.

If you think it is still relevant today, and needs to be done in the near future, you can comment to update the status, or just manually remove the no-issue-activity label.

You can also confidently close this issue as not planned to keep our backlog clean.
Don't worry if you think the issue is still valuable to continue in the future.
It's searchable and can be reopened when it's time. 😄

@xiangjinwu
Copy link
Contributor

xiangjinwu commented Oct 22, 2024

Issue located:

The psycopg3 client requires a ParameterStatus message of TimeZone to initialize the following:
https://github.com/psycopg/psycopg/blob/pool-3.2.3/psycopg_c/psycopg_c/types/datetime.pyx#L697

But RisingWave only sends a subset of parameters compared to PostgreSQL:

At present there is a hard-wired set of parameters for which ParameterStatus will be generated: they are server_version, server_encoding, client_encoding, application_name, default_transaction_read_only, in_hot_standby, is_superuser, session_authorization, DateStyle, IntervalStyle, TimeZone, integer_datetimes, and standard_conforming_strings.

fn write_parameter_status_msg_no_flush(&mut self, status: &ParameterStatus) -> io::Result<()> {
self.write_no_flush(&BeMessage::ParameterStatus(
BeParameterStatusMessage::ClientEncoding(SERVER_ENCODING),
))?;
self.write_no_flush(&BeMessage::ParameterStatus(
BeParameterStatusMessage::StandardConformingString(STANDARD_CONFORMING_STRINGS),
))?;
self.write_no_flush(&BeMessage::ParameterStatus(
BeParameterStatusMessage::ServerVersion(PG_VERSION),
))?;
if let Some(application_name) = &status.application_name {
self.write_no_flush(&BeMessage::ParameterStatus(
BeParameterStatusMessage::ApplicationName(application_name),
))?;
}
Ok(())
}

Extra notes:

  • Other clients proactively set their preferred timezone rather than waiting for the server's default timezone.
  • psycopg3 has some logic about defaulting to UTC but somehow it still segfaults due to uninitialized value.

@xiangjinwu xiangjinwu changed the title failed to fetch TIMESTAMPTZ data using psycopg3 bug(pgwire): failed to fetch TIMESTAMPTZ data using psycopg3 Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants