public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v1 2/6] ci: Add CI for FreeBSD, Linux, MacOS and Windows, utilizing cirrus-ci.
3+ messages / 2 participants
[nested] [flat]

* [PATCH v1 2/6] ci: Add CI for FreeBSD, Linux, MacOS and Windows, utilizing cirrus-ci.
@ 2021-03-15 16:25 Andres Freund <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Andres Freund @ 2021-03-15 16:25 UTC (permalink / raw)

---
 .cirrus.yml                     | 306 ++++++++++++++++++++++++++++++++
 .dockerignore                   |   3 +
 ci/docker/linux_debian_bullseye |   9 +
 ci/docker/windows_vs_2019       |  82 +++++++++
 ci/freebsd_gcp_repartition.sh   |  28 +++
 ci/pg_ci_base.conf              |  12 ++
 ci/windows_build_config.pl      |   3 +
 7 files changed, 443 insertions(+)
 create mode 100644 .cirrus.yml
 create mode 100644 .dockerignore
 create mode 100644 ci/docker/linux_debian_bullseye
 create mode 100644 ci/docker/windows_vs_2019
 create mode 100755 ci/freebsd_gcp_repartition.sh
 create mode 100644 ci/pg_ci_base.conf
 create mode 100644 ci/windows_build_config.pl

diff --git a/.cirrus.yml b/.cirrus.yml
new file mode 100644
index 00000000000..9f513f7d10a
--- /dev/null
+++ b/.cirrus.yml
@@ -0,0 +1,306 @@
+env:
+  # accelerate initial clone, but a bit of depth so that concurrent tasks work
+  CIRRUS_CLONE_DEPTH: 100
+  # Useful to be able to analyse what in a script takes long
+  CIRRUS_LOG_TIMESTAMP: true
+  # target to test, for all but windows
+  CHECK: check-world
+  CHECKFLAGS: -Otarget
+  PGCTLTIMEOUT: 120
+  CCACHE_MAXSIZE: "500M"
+  TEMP_CONFIG: ${CIRRUS_WORKING_DIR}/ci/pg_ci_base.conf
+
+
+task:
+  name: FreeBSD
+  only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*freebsd.*'
+  compute_engine_instance:
+    image_project: pg-vm-images-aio
+    image: family/pg-aio-freebsd-13-0
+    platform: freebsd
+    cpu: 2
+    memory: 2G
+    disk: 50
+  env:
+    CCACHE_DIR: "/tmp/ccache_dir"
+
+  ccache_cache:
+    folder: "/tmp/ccache_dir"
+  sysinfo_script:
+    - export || true
+  sysconfig_script:
+    - sudo sysctl kern.corefile='/tmp/%N.%P.core'
+  repartition_script:
+    - ci/freebsd_gcp_repartition.sh
+  create_user_script:
+    - pw useradd postgres
+    - chown -R postgres:postgres .
+    - mkdir -p /tmp/ccache_dir
+    - chown -R postgres:postgres /tmp/ccache_dir
+
+  configure_script:
+    - su postgres -c './configure --enable-cassert --enable-debug --enable-tap-tests --with-includes=/usr/local/include --with-libs=/usr/local/lib CC="ccache cc"'
+  build_script:
+    - su postgres -c 'gmake -s -j3 && gmake -s -j3 -C contrib'
+  upload_caches:
+    - ccache
+
+  check_world_script:
+    - su postgres -c 'time gmake -s -j2 ${CHECK} ${CHECKFLAGS}'
+
+  on_failure:
+    core_script:
+      for corefile in $(find /tmp -name '*.core' 2>/dev/null) ; do binary=$(gdb -quiet -core $corefile -batch -ex 'info auxv' | grep AT_EXECPATH | perl -pe "s/^.*\"(.*)\"\$/\$1/g") ; echo dumping $corefile for $binary ; gdb --batch --quiet -ex "thread apply all bt full" -ex "quit" $binary $corefile ; done
+    log_artifacts:
+      path: "**/**.log"
+      type: text/plain
+    regress_diffs_artifacts:
+      path: "**/**.diffs"
+      type: text/plain
+    tap_artifacts:
+      path: "**/regress_log_*"
+      type: text/plain
+
+
+task:
+  name: Linux
+  only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*linux.*'
+  compute_engine_instance:
+    image_project: pg-vm-images-aio
+    image: family/pg-aio-bullseye
+    platform: linux
+    cpu: 4
+    memory: 8G
+    nested_virtualization: false
+  env:
+    CCACHE_DIR: "/tmp/ccache_dir"
+    DEBUGINFOD_URLS: "https://debuginfod.debian.net";
+    TIMEOUT_CMD: timeout -s KILL -v 25m
+  ccache_cache:
+    folder: "/tmp/ccache_dir"
+
+  sysinfo_script:
+    - id
+    - uname -a
+    - cat /proc/cmdline
+    - lsblk
+    - ulimit -a -H
+    - ulimit -a -S
+    - export
+  sysconfig_script:
+    - useradd -m postgres
+    - chown -R postgres:postgres .
+    - mkdir -p /tmp/ccache_dir
+    - chown -R postgres:postgres /tmp/ccache_dir
+    - echo '* - memlock 134217728' > /etc/security/limits.d/postgres.conf
+    - su postgres -c 'ulimit -l -H'
+    - su postgres -c 'ulimit -l -S'
+    - echo '/tmp/%e-%s-%p.core' > /proc/sys/kernel/core_pattern
+
+  configure_script:
+    - echo "COPT=-O0 -ggdb" > src/Makefile.custom
+    - su postgres -c './configure --enable-cassert --enable-debug --enable-tap-tests --with-tcl --with-python --with-perl --with-ldap --with-openssl --with-icu --with-llvm CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"'
+  build_script:
+    - su postgres -c 'make -s -j4 && make -s -j4 -C contrib'
+  upload_caches:
+    - ccache
+
+  tests_script:
+    - su postgres -c 'ulimit -c unlimited ; ${TIMEOUT_CMD} make -s ${CHECK} ${CHECKFLAGS} -j8'
+
+  on_failure:
+    cores_script:
+      - for corefile in $(find /tmp/ -name '*.core' 2>/dev/null) ; do binary=$(gdb -quiet -core $corefile -batch -ex 'info auxv' | grep AT_EXECFN | perl -pe "s/^.*\"(.*)\"\$/\$1/g") ; echo dumping $corefile for $binary ; gdb --batch --quiet -ex "thread apply all bt full" -ex "quit" $binary $corefile ; done
+    log_artifacts:
+      path: "**/**.log"
+      type: text/plain
+    regress_diffs_artifacts:
+      path: "**/**.diffs"
+      type: text/plain
+    tap_artifacts:
+      path: "**/regress_log_*"
+      type: text/plain
+
+
+task:
+  name: macOS
+  only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*(macos|darwin|osx).*'
+  osx_instance:
+    image: big-sur-base
+  env:
+    CIRRUS_WORKING_DIR: ${HOME}/pgsql/
+    TEMP_CONFIG: ${CIRRUS_WORKING_DIR}/ci/pg_ci_base.conf
+    CCACHE_DIR: ${HOME}/ccache
+    HOMEBREW_CACHE: ${HOME}/homebrew-cache
+    PERL5LIB: ${HOME}/perl5/lib/perl5
+
+  sysinfo_script:
+    - id
+    - export
+  ccache_cache:
+    folder: ${CCACHE_DIR}
+  homebrew_cache:
+    folder: ${HOMEBREW_CACHE}
+  perl_cache:
+    folder: ~/perl5
+
+  cpan_install_script:
+    - perl -mIPC::Run -e 1 || cpan -T IPC::Run
+    - perl -mIO::Pty -e 1 || cpan -T IO::Pty
+  upload_caches:
+    - perl
+  core_install_script:
+    - sudo chmod 777 /cores
+  homebrew_install_script:
+    - brew install make coreutils ccache
+  upload_caches:
+    - homebrew
+
+  configure_script:
+    - ./configure --prefix=$HOME/install --enable-cassert --enable-debug --enable-tap-tests CC="ccache gcc" CFLAGS="-O0"
+  build_script:
+    - gmake -s -j12 && gmake -s -j12 -C contrib
+  upload_caches:
+    - ccache
+
+  tests_script:
+    - ulimit -c unlimited
+    - ulimit -n 1024
+    - gtimeout -s KILL -v 20m gmake -s -j12 ${CHECK} ${CHECKFLAGS}
+
+  on_failure:
+    cores_script:
+      - for corefile in $(find /cores/ -name 'core.*' 2>/dev/null) ; do lldb -c $corefile --batch -o 'thread backtrace all' -o 'quit' ; done
+    log_artifacts:
+      path: "**/**.log"
+      type: text/plain
+    regress_diffs_artifacts:
+      path: "**/**.diffs"
+      type: text/plain
+    tap_artifacts:
+      path: "**/regress_log_*"
+      type: text/plain
+
+
+task:
+  name: Windows
+  only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*windows.*'
+  windows_container:
+    dockerfile: ci/docker/windows_vs_2019
+    cpu: 4
+    memory: 4G
+  env:
+    PROVE_FLAGS: -j10
+    # The default working dir is in a directory msbuild complains about
+    CIRRUS_WORKING_DIR: "c:/cirrus"
+    TEMP_CONFIG: ${CIRRUS_WORKING_DIR}/ci/pg_ci_base.conf
+    # Avoid re-installing over and over
+    NO_TEMP_INSTALL: 1
+
+  sysinfo_script:
+    - chcp
+    - systeminfo
+    - powershell -Command get-psdrive -psprovider filesystem
+    - ps: Get-Item -Path 'HKLM:\SOFTWARE\Microsoft\Windows NT\CurrentVersion\AeDebug'
+    - set
+
+  configure_script:
+    - copy ci\windows_build_config.pl src\tools\msvc\config.pl
+    - vcvarsall x64
+    - perl src/tools/msvc/mkvcbuild.pl
+  build_script:
+    - vcvarsall x64
+    # Disable file tracker, we're never going to rebuild...
+    - msbuild -m /p:TrackFileAccess=false pgsql.sln
+  tempinstall_script:
+    - perl src\tools\msvc\install.pl tmp_install
+
+  check_script:
+    - perl src/tools/msvc/vcregress.pl check parallel
+  startcreate_script:
+    - tmp_install\bin\pg_ctl.exe initdb -D tmp_check\db -l tmp_check\initdb.log
+    - echo include '%TEMP_CONFIG%' >> tmp_check\db\postgresql.conf
+    - tmp_install\bin\pg_ctl.exe start -D tmp_check\db -l tmp_check\postmaster.log
+  isolationcheck_script:
+    - perl src/tools/msvc/vcregress.pl isolationcheck
+  modulescheck_script:
+    - perl src/tools/msvc/vcregress.pl modulescheck
+  contribcheck_script:
+    - perl src/tools/msvc/vcregress.pl contribcheck
+  plcheck_script:
+    - perl src/tools/msvc/vcregress.pl plcheck
+  stop_script:
+    - tmp_install\bin\pg_ctl.exe stop -D tmp_check\db -l tmp_check\postmaster.log
+  subscriptioncheck_script:
+    - perl src/tools/msvc/vcregress.pl taptest .\src\test\subscription\
+  authentication_script:
+    - perl src/tools/msvc/vcregress.pl taptest .\src\test\authentication\
+  recoverycheck_script:
+    - perl src/tools/msvc/vcregress.pl recoverycheck
+  bincheck_script:
+    - perl src/tools/msvc/vcregress.pl bincheck
+  upgradecheck_script:
+    - perl src/tools/msvc/vcregress.pl upgradecheck
+  ecpgcheck_script:
+    # tries to build additional stuff
+    - vcvarsall x64
+    # References ecpg_regression.proj in the current dir
+    - cd src\tools\msvc
+    - perl vcregress.pl ecpgcheck
+
+  always:
+    cat_dumps_script:
+      - cat crashlog.txt || true
+    dump_artifacts:
+      path: "crashlog.txt"
+      type: text/plain
+
+  on_failure:
+    log_artifacts:
+      path: "**/**.log"
+      type: text/plain
+    regress_diffs_artifacts:
+      path: "**/**.diffs"
+      type: text/plain
+    tap_artifacts:
+      path: "**/regress_log_*"
+      type: text/plain
+
+
+task:
+  name: CompilerWarnings
+  depends_on:
+  - Linux
+  # not run task count as a success, so we need to recheck Linux' condition here :/
+  only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*linux.*'
+  container:
+    dockerfile: ci/docker/linux_debian_bullseye
+  env:
+    CCACHE_SIZE: "4GB"
+    CCACHE_DIR: "/tmp/ccache_dir"
+  ccache_cache:
+    folder: "/tmp/ccache_dir"
+  setup_script:
+    - echo "COPT=-Werror" > src/Makefile.custom
+    - gcc -v
+    - clang -v
+  # gcc with asserts disabled
+  always:
+    gcc_warning_script:
+      - ./configure --cache gcc.cache CC="ccache gcc"
+      - time make -s -j4 clean && time make -s -j4
+  # gcc with asserts enabled
+  always:
+    gcc_a_warning_script:
+      - ./configure --cache gcc.cache --enable-cassert CC="ccache gcc"
+      - time make -s -j4 clean && time make -s -j4
+  # clang with asserts disabled
+  always:
+    clang_warning_script:
+      - ./configure --cache clang.cache CC="ccache clang"
+      - time make -s -j4 clean && time make -s -j4
+  # clang with asserts enabled
+  always:
+    clang_a_warning_script:
+      - ./configure --cache clang.cache --enable-cassert CC="ccache clang"
+      - time make -s -j4 clean && time make -s -j4
diff --git a/.dockerignore b/.dockerignore
new file mode 100644
index 00000000000..3fceab2e97b
--- /dev/null
+++ b/.dockerignore
@@ -0,0 +1,3 @@
+# Ignore everything, except ci/
+*
+!ci/*
diff --git a/ci/docker/linux_debian_bullseye b/ci/docker/linux_debian_bullseye
new file mode 100644
index 00000000000..3c1d1a4583a
--- /dev/null
+++ b/ci/docker/linux_debian_bullseye
@@ -0,0 +1,9 @@
+FROM debian:bullseye
+RUN apt-get -y update && apt-get -y upgrade && \
+  DEBIAN_FRONTEND=noninteractive apt-get -y --no-install-recommends install \
+      gcc g++ libreadline-dev flex bison make perl libipc-run-perl clang \
+      llvm-dev libperl-dev libpython3-dev tcl-dev libldap2-dev libicu-dev \
+      docbook-xml docbook-xsl fop libxml2-utils xsltproc krb5-admin-server \
+      krb5-kdc krb5-user slapd ldap-utils libssl-dev pkg-config locales-all \
+      liburing-dev python3-distutils ccache gdb && \
+  apt-get clean
diff --git a/ci/docker/windows_vs_2019 b/ci/docker/windows_vs_2019
new file mode 100644
index 00000000000..e5b4810ac9e
--- /dev/null
+++ b/ci/docker/windows_vs_2019
@@ -0,0 +1,82 @@
+# escape=`
+
+# We used to use the visual studio container, but it's too outdated now
+FROM cirrusci/windowsservercore:2019
+
+SHELL ["powershell", "-NoLogo", "-NoProfile", "-Command"]
+
+
+RUN `
+    New-Item -Path 'HKLM:\SOFTWARE\Policies\Microsoft\VisualStudio' ; `
+    New-Item -Path 'HKLM:\SOFTWARE\Policies\Microsoft\VisualStudio\Setup' ; `
+    New-ItemProperty -Path 'HKLM:\SOFTWARE\Policies\Microsoft\VisualStudio\Setup' -Name KeepDownloadedPayloads -Value 0 -PropertyType DWord
+
+
+# Install commandline debugger and log all crashes to c:\cirrus\crashlog.txt
+#
+# Done manually as doing this via chocolatey / the installer directly, ends up
+# with a lot of unnecessary chaff, making the layer unnecessarily large.
+RUN `
+    mkdir c:\t ; `
+    cd c:\t ; `
+    curl.exe -sSL -o 'windsdksetup.exe' https://download.microsoft.com/download/9/7/9/97982c1d-d687-41be-9dd3-6d01e52ceb68/windowssdk/winsdk... ; `
+    Start-Process -FilePath ".\windsdksetup.exe" `
+      -ArgumentList '/Features OptionId.WindowsDesktopDebuggers /layout c:\t\sdk /quiet /norestart /log c:\t\sdk.log' `
+    -Wait  ; `
+    `
+    Start-Process -FilePath msiexec.exe `
+      -ArgumentList '/a \"C:\t\sdk\Installers\X64 Debuggers And Tools-x64_en-us.msi\" /qb /log install2.log' `
+    -Wait ; `
+    C:\Windows` Kits\10\Debuggers\x64\cdb.exe -version ; `
+    `
+    cd c:\ ; `
+    Remove-Item C:\t\* -Force -Recurse ; `
+    Remove-Item C:\t -Force -Recurse ; `
+    Set-ItemProperty -Path 'HKLM:\SOFTWARE\Microsoft\Windows NT\CurrentVersion\AeDebug' -Name 'Debugger' -Value '\"C:\Windows Kits\10\Debuggers\x64\cdb.exe\" -p %ld -e %ld -g -kqm -c \".lines -e; .symfix+ ;.logappend c:\cirrus\crashlog.txt ; !peb; ~*kP ; .logclose ; q \"' ; `
+    New-ItemProperty -Path 'HKLM:\SOFTWARE\Microsoft\Windows NT\CurrentVersion\AeDebug' -Name 'Auto' -Value 1 -PropertyType DWord ; `
+    Get-ItemProperty -Path 'HKLM:\SOFTWARE\Microsoft\Windows NT\CurrentVersion\AeDebug' -Name Debugger; `
+    setx PATH \"C:\Windows Kits\10\Debuggers\x64;$Env:PATH\" /m
+
+
+# Install perl and bison.
+#
+# Done manually as choco takes a lot longer. I think it's download issues with
+# powershell's download stuff? That's wy curl.exe is directly used here at least...
+RUN `
+    mkdir c:\t ; `
+    cd c:\t ; `
+    `
+    curl.exe -sSL -o perl.zip `
+        https://strawberryperl.com/download/5.30.3.1/strawberry-perl-5.30.3.1-64bit-portable.zip ; `
+    7z.exe x .\perl.zip -xr!c -oc:\strawberry ; `
+    `
+    curl.exe -sSL -o winflexbison.zip `
+        https://github.com/lexxmark/winflexbison/releases/download/v2.5.24/win_flex_bison-2.5.24.zip ; `
+    7z.exe x .\winflexbison.zip -oc:\winflexbison ; `
+    Rename-Item -Path c:\winflexbison\win_flex.exe c:\winflexbison\flex.exe ; `
+    Rename-Item -Path c:\winflexbison\win_bison.exe c:\winflexbison\bison.exe ; `
+    `
+    cd c:\ ; `
+    Remove-Item C:\t -Force -Recurse ; `
+    setx PATH \"C:\strawberry\perl\bin;C:\winflexbison;C:\Program Files\Git\usr\bin;$Env:PATH\" /m
+
+
+# Install visual studio
+#
+# Adding VS path to vcvarsall.bat so user of container doesn't need to know the full path
+RUN `
+    mkdir c:\t ; `
+    cd c:\t ; `
+    curl.exe -sSL -o c:\t\vs_buildtools.exe https://aka.ms/vs/16/release/vs_buildtools.exe ; `
+    Start-Process -Wait `
+        -FilePath c:\t\vs_buildtools.exe `
+        -ArgumentList `
+          '--quiet', '--wait', '--norestart', '--nocache', `
+          '--installPath', 'c:\BuildTools', `
+          '--add', 'Microsoft.VisualStudio.Component.VC.Tools.x86.x64', `
+          '--add', 'Microsoft.VisualStudio.Component.Windows10SDK.20348'  ; `
+    cd c:\ ; `
+    Remove-Item C:\t -Force -Recurse ; `
+    Remove-Item -Force -Recurse ${Env:TEMP}\*; `
+    Remove-Item -Force -Recurse \"${Env:ProgramData}\Package Cache\"  ; `
+    setx PATH \"c:\BuildTools\VC\Auxiliary\Build;$Env:PATH\" /m
diff --git a/ci/freebsd_gcp_repartition.sh b/ci/freebsd_gcp_repartition.sh
new file mode 100755
index 00000000000..2d5e1738998
--- /dev/null
+++ b/ci/freebsd_gcp_repartition.sh
@@ -0,0 +1,28 @@
+#!/bin/sh
+
+set -e
+set -x
+
+# The default filesystem on freebsd gcp images is very slow to run tests on,
+# due to its 32KB block size
+#
+# XXX: It'd probably better to fix this in the image, using something like
+# https://people.freebsd.org/~lidl/blog/re-root.html
+
+# fix backup partition table after resize
+gpart recover da0
+gpart show da0
+# kill swap, so we can delete a partition
+swapoff -a || true
+# (apparently we can only have 4!?)
+gpart delete -i 3 da0
+gpart add -t freebsd-ufs -l data8k -a 4096 da0
+gpart show da0
+newfs -U -b 8192 /dev/da0p3
+
+# Migrate working directory
+du -hs $CIRRUS_WORKING_DIR
+mv $CIRRUS_WORKING_DIR $CIRRUS_WORKING_DIR.orig
+mkdir $CIRRUS_WORKING_DIR
+mount -o noatime /dev/da0p3 $CIRRUS_WORKING_DIR
+cp -r $CIRRUS_WORKING_DIR.orig/* $CIRRUS_WORKING_DIR/
diff --git a/ci/pg_ci_base.conf b/ci/pg_ci_base.conf
new file mode 100644
index 00000000000..637e3cfb343
--- /dev/null
+++ b/ci/pg_ci_base.conf
@@ -0,0 +1,12 @@
+# Tends to produce too many core files, taking a long time
+restart_after_crash = false
+
+# So that tests using the "manually" started postgres on windows can use
+# prepared statements
+max_prepared_transactions=10
+
+# Settings that make logs more useful
+log_line_prefix='%m [%p][%b][%v:%x] '
+log_checkpoints = true
+log_connections = true
+log_disconnections = true
diff --git a/ci/windows_build_config.pl b/ci/windows_build_config.pl
new file mode 100644
index 00000000000..a8abfa2a02f
--- /dev/null
+++ b/ci/windows_build_config.pl
@@ -0,0 +1,3 @@
+$config->{"tap_tests"} = 1;
+$config->{"asserts"} = 1;
+1;
-- 
2.32.0.rc2


--ntohwnblr5nw7rqz
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v1-0003-Reference-test-binary-using-TESTDIR-in-001_libpq_.patch"



^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: ALTER TABLE SET ACCESS METHOD on partitioned tables
@ 2023-03-20 00:30 Michael Paquier <[email protected]>
  2023-03-28 00:13 ` Re: ALTER TABLE SET ACCESS METHOD on partitioned tables Michael Paquier <[email protected]>
  0 siblings, 1 reply; 3+ messages in thread

From: Michael Paquier @ 2023-03-20 00:30 UTC (permalink / raw)
  To: Soumyadeep Chakraborty <[email protected]>; +Cc: Zhihong Yu <[email protected]>; Justin Pryzby <[email protected]>; pgsql-hackers; Ashwin Agrawal <[email protected]>; [email protected]

On Thu, Jun 09, 2022 at 11:21:58AM -0700, Soumyadeep Chakraborty wrote:
> Thanks. Fixed and rebased.

I think that I am OK with the concept of this patch to use a
partitioned table's relam as a reference when creating a partition
rather than relying on the default GUC, in a way similar to
tablespaces.

One worry I see is that forcing a recursion on the leaves on ALTER
TABLE could silently break partitions where multiple table AMs are
used across separate partitions if ALTER TABLE SET ACCESS METHOD is
used on one of the parents, though it seems like this is not something
I would much worry about as now the command is an error.

A second worry is that we would just break existing creation flows
that rely on the GUC defining the default AM.  This is worth a close
lookup at pg_dump to make sure that we do things correctly with this
patch in place..  Did you check dump and restore flows with partition
trees and --no-table-access-method?  Perhaps there should be
some regression tests with partitioned tables?

+    /*
+     * For partitioned tables, when no access method is specified, we
+     * default to the parent table's AM.
+     */
+    Assert(list_length(inheritOids) == 1);
+    /* XXX: should implement get_rel_relam? */
+    relid = linitial_oid(inheritOids);
+    tup = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
+    if (!HeapTupleIsValid(tup))
+            elog(ERROR, "cache lookup failed for relation %u", relid);

Having a wrapper for that could be useful, yes.  We don't have any
code paths that would directly need that now, from what I can see,
though.  This patch gives one reason to have one.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: ALTER TABLE SET ACCESS METHOD on partitioned tables
  2023-03-20 00:30 Re: ALTER TABLE SET ACCESS METHOD on partitioned tables Michael Paquier <[email protected]>
@ 2023-03-28 00:13 ` Michael Paquier <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Michael Paquier @ 2023-03-28 00:13 UTC (permalink / raw)
  To: Soumyadeep Chakraborty <[email protected]>; +Cc: Zhihong Yu <[email protected]>; Justin Pryzby <[email protected]>; pgsql-hackers; Ashwin Agrawal <[email protected]>; [email protected]

On Mon, Mar 20, 2023 at 09:30:50AM +0900, Michael Paquier wrote:
> Did you check dump and restore flows with partition
> trees and --no-table-access-method?  Perhaps there should be
> some regression tests with partitioned tables?

I was looking at the patch, and as I suspected the dumps generated
are forgetting to apply the AM to the partitioned tables.  For
example, assuming that the default AM is heap, the patch creates
child_0_10 with heap2 as AM, which is what we want:
CREATE ACCESS METHOD heap2 TYPE TABLE HANDLER heap_tableam_handler;
CREATE TABLE parent_tab (id int) PARTITION BY RANGE (id) USING heap2;
CREATE TABLE child_0_10 PARTITION OF parent_tab
  FOR VALUES FROM (0) TO (10);

However a dump produces that (content cut except for its most relevant
parts):
CREATE ACCESS METHOD heap2 TYPE TABLE HANDLER heap_tableam_handler;
SET default_tablespace = '';
CREATE TABLE public.parent_tab (
    id integer
)
PARTITION BY RANGE (id);
SET default_table_access_method = heap2;
CREATE TABLE public.child_0_10 (
    id integer
);

This would restore the previous contents incorrectly, where parent_tab
would use heap and child_0_10 would use heap2, causing any partitions
created after the restore to use silently heap.  This is going to
require a logic similar to tablespaces, where generate SET commands
on default_table_access_method so as --no-table-access-method in
pg_dump and pg_restore are able to work correctly.  Having tests to
cover all that is a must-have.
--
Michael


Attachments:

  [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 3+ messages in thread


end of thread, other threads:[~2023-03-28 00:13 UTC | newest]

Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-03-15 16:25 [PATCH v1 2/6] ci: Add CI for FreeBSD, Linux, MacOS and Windows, utilizing cirrus-ci. Andres Freund <[email protected]>
2023-03-20 00:30 Re: ALTER TABLE SET ACCESS METHOD on partitioned tables Michael Paquier <[email protected]>
2023-03-28 00:13 ` Re: ALTER TABLE SET ACCESS METHOD on partitioned tables Michael Paquier <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox