feat: upgrade pgai extension to 0.7.0 #291

smoya · 2025-01-20T10:04:27Z

No description provided.

CLAassistant · 2025-01-20T10:04:36Z

All committers have signed the CLA.

CLAassistant · 2025-01-20T10:04:37Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

smoya · 2025-01-20T15:55:41Z

Dockerfile

@@ -74,13 +74,15 @@ RUN set -ex; \
            python3-dev \
            apache-arrow-dev \
            py3-pip; \
+        # Upgrade pip to 24.3.1 to match the version in the ha timescaledb-docker image.
+        python3 -m pip install --upgrade pip==24.3.1 --break-system-packages; \


Matches with https://github.com/timescale/timescaledb-docker-ha/pull/514/files#diff-dd2c0eb6ea5cfc6c4bd4eac30934e2d5746747af48fef6da689e85b752f39557R153

Without --break-system-packages, it complains about being under a managed python venv. Not sure if this is the really idiomatic way to go or rather I should activate the venv somehow from here.

smoya · 2025-01-20T15:58:56Z

Makefile

@@ -5,7 +5,7 @@ ORG=timescaledev
 PG_VER=pg16
 PG_VER_NUMBER=$(shell echo $(PG_VER) | cut -c3-)
 PG_MAJOR_VERSION=$(shell echo $(PG_VER_NUMBER) | cut -d. -f1)
-ifeq ($(shell test $(PG_MAJOR_VERSION) -ge 17; echo $$?),0)
+ifeq ($(shell test $(PG_MAJOR_VERSION) -ge 16; echo $$?),0)


What are the implications on this change? Without it, errors in pg16 build appear such as:

#16 53.42 In file included from /tmp/pip-install-ylxps7gq/pyarrow_996747a81b7645e4b8ae6232db0fd67b/pyarrow/src/arrow/python/arrow_to_pandas.cc:54: #16 53.42 /tmp/pip-install-ylxps7gq/pyarrow_996747a81b7645e4b8ae6232db0fd67b/pyarrow/src/arrow/python/decimal.h:66:33: error: 'Decimal32' has not been declared

I've tried running the following test in order to understand why exactly this change suggested fixes the issue but didn't have any success yet:

Test 1: Upgrade only to alpine 3.21, keep same clang version.

Both Failing since clang19 is somehow required - Details.

Test 2. Keep same alpine version, upgrade clang version (to 18, since 19 is not available in alpine 3.20)

PG16: make: clang-15: No such file or directory . Details.

PG17: Error on apk add postgres: 1.128 postgresql17-plpython3 . Details.

I still don't understand why the compilation of pgvector asks for a clang version different than the actual specified in the Makefile 🤔. Is it Postgres who's expecting such a clang version? It seems so since the error message comes from executing a Makefile from Postgres source.

Unless we find a better alternative, upgrading to Alpine 3.21 seems like a plan. However, I believe we are not using such a version in pg16 because of a reason.

Perhaps @svenklemm can share more insights 🙏

Changing alpine version will require some adjustments in timescaledb CI but I think upgrading alpine is lesser evil than not having to adjust our ci. We pinned the alpine version to make backwards compat checks easier. This was causing problems cause the different alpine versions have their ssl stuff in different packages. But latest timescaledb versions are not affected anyway as we changed the linking to no longer link directly against ssl but indirectly through postgres.

For the record, and for future readers, the reason why the error about libarrow is gone when upgrading to Alpine v3.21 is because the libarrow package on such an Alpine version contains headers that are compatible with the required pgai pyarrow dep (requires 19.0.0), meanwhile those headers of the libarrow package installed in Alpine v3.20 aren't compatible.

Thanks @JamesGuthrie for pointing that out 🙏 💯

@svenklemm are you ok if we set the upgrade of alpine version here in this PR (as it is now) or want me do create a separate PR for that instead and merge it before merging this one?

it's fine to do in this one

smoya · 2025-01-21T09:57:01Z

Dockerfile

        git clone --branch ${PGAI_VERSION} https://github.com/timescale/pgai.git /build/pgai; \
        cd /build/pgai; \
        # note: this is a hack. pyarrow will be built from source, so must be pinned to this arrow version \
-        echo pyarrow==$(pkg-config --modversion arrow) >> ./projects/extension/requirements.txt; \
+        echo pyarrow==$(pkg-config --modversion arrow) >> ./projects/extension/requirements-lock.txt; \


We moved from having a requirements.txt file to requirements-lock.txt file with locked dependencies (built by uv).
Even though without this change the install() from build.py will consider deps in requirements.txt to be additionally installed (See install_old_py_deps(), this change is necessary for consistency sake; If tomorrow we stop considering installing old dependencies from requirements.txt file, this will fail.

smoya · 2025-01-21T17:18:27Z

Dockerfile

        if [ "$TARGETARCH" == "386" ]; then \
            # note: pinned because pandas 2.2.0-2.2.3 on i386 is affected by https://github.com/pandas-dev/pandas/issues/59905 \
-            echo pandas==2.1.4 >> ./projects/extension/requirements.txt; \
+            sed -i '/dependencies= \[/a \    "pandas==2.1.4",' projects/extension/pyproject.toml; \


Ended up adding the dependency to the pyproject.toml file as pip install is not considering requirements-lock.txt.

cc @JamesGuthrie @jgpruitt @Askir

An alternative option (which doesn't require hacking around in the pyproject.toml) is to create a contstraints.txt file and get pip to use that (by setting the PIP_CONSTRAINT env var):

echo "pandas==2.1.4" > constraints.txt export PIP_CONSTRAINT=$(pwd)/constraints.txt

An alternative option (which doesn't require hacking around in the pyproject.toml) is to create a contstraints.txt file and get pip to use that (by setting the PIP_CONSTRAINT env var):

echo "pandas==2.1.4" > constraints.txt export PIP_CONSTRAINT=$(pwd)/constraints.txt

TIL! 🧠

I actually like your solution with constraints more than the hack because it exactly does what we need.
Changing it right now. Thanks for such a great suggestion @JamesGuthrie

YIL (yesterday I learned!) I also like it more than what we did before.

smoya · 2025-01-21T17:36:34Z

Dockerfile

        git clone --branch ${PGAI_VERSION} https://github.com/timescale/pgai.git /build/pgai; \
        cd /build/pgai; \
-        # note: this is a hack. pyarrow will be built from source, so must be pinned to this arrow version \
-        echo pyarrow==$(pkg-config --modversion arrow) >> ./projects/extension/requirements.txt; \


This is not needed anymore since we get the headers from the apache-arrow-dev package and install from source using them. Am I right or perhaps missing something @JamesGuthrie ?

This is still needed. Adding this to the requirements ensures that we install the version of pyarrow which corresponds to the version of libarrow which is available on the current platform.

This is still needed. Adding this to the requirements ensures that we install the version of pyarrow which corresponds to the version of libarrow which is available on the current platform.

Good point 💯

cevian

I am fine with these changes but we should make pyarrow global very soon

Signed-off-by: Sergio Moya <[email protected]>

smoya force-pushed the feat/upgradePgai branch 7 times, most recently from 67622e8 to 892220a Compare January 20, 2025 15:52

smoya commented Jan 20, 2025

View reviewed changes

smoya commented Jan 21, 2025

View reviewed changes

adolsalamanca approved these changes Jan 21, 2025

View reviewed changes

smoya force-pushed the feat/upgradePgai branch 3 times, most recently from 73b034b to 515201a Compare January 21, 2025 11:08

svenklemm approved these changes Jan 21, 2025

View reviewed changes

smoya force-pushed the feat/upgradePgai branch 2 times, most recently from f30d35d to a84c66a Compare January 21, 2025 17:14

smoya commented Jan 21, 2025

View reviewed changes

smoya force-pushed the feat/upgradePgai branch from a84c66a to c558cf5 Compare January 21, 2025 17:35

smoya commented Jan 21, 2025

View reviewed changes

adolsalamanca approved these changes Jan 21, 2025

View reviewed changes

cevian approved these changes Jan 21, 2025

View reviewed changes

smoya force-pushed the feat/upgradePgai branch from c558cf5 to 1b86dad Compare January 22, 2025 09:45

eat: upgrade pgai extension to 0.7.0

d5ce28e

Signed-off-by: Sergio Moya <[email protected]>

smoya force-pushed the feat/upgradePgai branch from 1b86dad to d5ce28e Compare January 22, 2025 09:51

smoya merged commit d7dbb90 into main Jan 22, 2025
6 checks passed

smoya deleted the feat/upgradePgai branch January 22, 2025 10:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: upgrade pgai extension to 0.7.0 #291

feat: upgrade pgai extension to 0.7.0 #291

smoya commented Jan 20, 2025

CLAassistant commented Jan 20, 2025 •

edited

Loading

CLAassistant commented Jan 20, 2025

smoya Jan 20, 2025 •

edited

Loading

smoya Jan 20, 2025

smoya Jan 20, 2025 •

edited

Loading

smoya Jan 21, 2025

svenklemm Jan 21, 2025 •

edited

Loading

smoya Jan 21, 2025

smoya Jan 21, 2025 •

edited

Loading

svenklemm Jan 21, 2025

smoya Jan 21, 2025 •

edited

Loading

smoya Jan 21, 2025 •

edited

Loading

JamesGuthrie Jan 22, 2025

smoya Jan 22, 2025

JamesGuthrie Jan 22, 2025

smoya Jan 21, 2025

JamesGuthrie Jan 22, 2025

smoya Jan 22, 2025

cevian left a comment

feat: upgrade pgai extension to 0.7.0 #291

feat: upgrade pgai extension to 0.7.0 #291

Conversation

smoya commented Jan 20, 2025

CLAassistant commented Jan 20, 2025 • edited Loading

CLAassistant commented Jan 20, 2025

smoya Jan 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smoya Jan 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

svenklemm Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smoya Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smoya Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

smoya Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cevian left a comment

Choose a reason for hiding this comment

CLAassistant commented Jan 20, 2025 •

edited

Loading

smoya Jan 20, 2025 •

edited

Loading

smoya Jan 20, 2025 •

edited

Loading

svenklemm Jan 21, 2025 •

edited

Loading

smoya Jan 21, 2025 •

edited

Loading

smoya Jan 21, 2025 •

edited

Loading

smoya Jan 21, 2025 •

edited

Loading