Merge branch 'master' into hash-partitioning

Author: ankane

CHANGELOG.md

@@ -1,7 +1,8 @@
 ## 0.7.1 (unreleased)
 
 - Added experimental support for hash partitioning
-- Improved `analyze` and `unprep` for declarative partitioning
+- Fixed `analyze` analyzing partitions twice with declarative partitioning
+- Removed unnecessary query for `unprep` with declarative partitioning
 
 ## 0.7.0 (2025-05-26)
 

README.md

@@ -301,7 +301,9 @@ You can also use pgslice to reduce the size of a table without partitioning by c
 ```sh
 pgslice prep <table> --no-partition
 pgslice fill <table> --where "id > 1000" # use any conditions
+pgslice analyze <table>
 pgslice swap <table>
+pgslice fill <table> --where "id > 1000" --swapped
 ```
 
 ## Hash Partitioning

lib/pgslice/cli.rb

@@ -1,8 +1,10 @@
 module PgSlice
   class CLI < Thor
     class << self
-      attr_accessor :instance
+      attr_accessor :instance, :exit_on_failure
+      alias_method :exit_on_failure?, :exit_on_failure
     end
+    self.exit_on_failure = true
 
     include Helpers
 
@@ -13,10 +15,6 @@ class << self
 
     map %w[--version -v] => :version
 
-    def self.exit_on_failure?
-      ENV["PGSLICE_ENV"] != "test"
-    end
-
     def initialize(*args)
       PgSlice::CLI.instance = self
       $stdout.sync = true

lib/pgslice/table.rb

@@ -12,11 +12,19 @@ def to_s
     end
 
     def exists?
-      execute("SELECT COUNT(*) FROM pg_catalog.pg_tables WHERE schemaname = $1 AND tablename = $2", [schema, name]).first["count"].to_i > 0
+      query = <<~SQL
+        SELECT COUNT(*) FROM pg_catalog.pg_tables
+        WHERE schemaname = $1 AND tablename = $2
+      SQL
+      execute(query, [schema, name]).first["count"].to_i > 0
     end
 
     def columns
-      execute("SELECT column_name FROM information_schema.columns WHERE table_schema = $1 AND table_name = $2 AND is_generated = 'NEVER'", [schema, name]).map { |r| r["column_name"] }
+      query = <<~SQL
+        SELECT column_name FROM information_schema.columns
+        WHERE table_schema = $1 AND table_name = $2 AND is_generated = 'NEVER'
+      SQL
+      execute(query, [schema, name]).map { |r| r["column_name"] }
     end
 
     # http://www.dbforums.com/showthread.php?1667561-How-to-list-sequences-and-the-columns-by-SQL
@@ -41,7 +49,11 @@ def sequences
     end
 
     def foreign_keys
-      execute("SELECT pg_get_constraintdef(oid) FROM pg_constraint WHERE conrelid = $1::regclass AND contype ='f'", [quote_table]).map { |r| r["pg_get_constraintdef"] }
+      query = <<~SQL
+        SELECT pg_get_constraintdef(oid) FROM pg_constraint
+        WHERE conrelid = $1::regclass AND contype ='f'
+      SQL
+      execute(query, [quote_table]).map { |r| r["pg_get_constraintdef"] }
     end
 
     # https://stackoverflow.com/a/20537829
@@ -69,7 +81,11 @@ def primary_key
     end
 
     def index_defs
-      execute("SELECT pg_get_indexdef(indexrelid) FROM pg_index WHERE indrelid = $1::regclass AND indisprimary = 'f'", [quote_table]).map { |r| r["pg_get_indexdef"] }
+      query = <<~SQL
+        SELECT pg_get_indexdef(indexrelid) FROM pg_index
+        WHERE indrelid = $1::regclass AND indisprimary = 'f'
+      SQL
+      execute(query, [quote_table]).map { |r| r["pg_get_indexdef"] }
     end
 
     def quote_table
@@ -89,7 +105,11 @@ def trigger_name
     end
 
     def column_cast(column)
-      data_type = execute("SELECT data_type FROM information_schema.columns WHERE table_schema = $1 AND table_name = $2 AND column_name = $3", [schema, name, column])[0]["data_type"]
+      query = <<~SQL
+        SELECT data_type FROM information_schema.columns
+        WHERE table_schema = $1 AND table_name = $2 AND column_name = $3
+      SQL
+      data_type = execute(query, [schema, name, column])[0]["data_type"]
       data_type == "timestamp with time zone" ? "timestamptz" : "date"
     end
 
@@ -139,7 +159,11 @@ def fetch_comment
     end
 
     def fetch_trigger(trigger_name)
-      execute("SELECT obj_description(oid, 'pg_trigger') AS comment FROM pg_trigger WHERE tgname = $1 AND tgrelid = $2::regclass", [trigger_name, quote_table])[0]
+      query = <<~SQL
+        SELECT obj_description(oid, 'pg_trigger') AS comment FROM pg_trigger
+        WHERE tgname = $1 AND tgrelid = $2::regclass
+      SQL
+      execute(query, [trigger_name, quote_table])[0]
     end
 
     # legacy

test/pgslice_test.rb

@@ -33,10 +33,18 @@ def test_no_partition
     run_command "fill Posts"
     assert_equal 10000, count("Posts_intermediate")
 
+    assert_analyzed "Posts_intermediate" do
+      run_command "analyze Posts"
+    end
+
     run_command "swap Posts"
     assert !table_exists?("Posts_intermediate")
     assert table_exists?("Posts_retired")
 
+    assert_analyzed "Posts" do
+      run_command "analyze Posts --swapped"
+    end
+
     run_command "unswap Posts"
     assert table_exists?("Posts_intermediate")
     assert !table_exists?("Posts_retired")
@@ -158,11 +166,8 @@ def assert_period(period, column: "createdAt", trigger_based: false, tablespace:
     # insert into old table
     execute %!INSERT INTO "Posts" (#{quote_ident(column)}) VALUES ($1) RETURNING "Id"!, [now.iso8601]
 
-    run_command "analyze Posts"
-    # https://github.com/postgres/postgres/commit/375aed36ad83f0e021e9bdd3a0034c0c992c66dc
-    if server_version_num >= 150000
-      last_analyzed = execute("SELECT relname, last_analyze FROM pg_stat_user_tables WHERE relname LIKE 'Posts_%'")
-      assert_equal 4, last_analyzed.count { |v| v["last_analyze"] }
+    assert_analyzed "Posts%", 4 do
+      run_command "analyze Posts"
     end
 
     # TODO check sequence ownership
@@ -219,7 +224,9 @@ def assert_period(period, column: "createdAt", trigger_based: false, tablespace:
     assert_column partition_name, "updatedAt"
     assert_column new_partition_name, "updatedAt"
 
-    run_command "analyze Posts --swapped"
+    assert_analyzed "Posts%", 6 do
+      run_command "analyze Posts --swapped"
+    end
 
     # pg_stats_ext view available with Postgres 12+
     assert_statistics "Posts" if !trigger_based
@@ -275,7 +282,7 @@ def run_command(command, error: nil)
       puts
     end
     stdout, stderr = capture_io do
-      PgSlice::CLI.start("#{command} --url #{$url}".split(" "))
+      PgSlice::CLI.start("#{command} --url #{url}".split(" "))
     end
     if verbose?
       puts stdout
@@ -360,6 +367,16 @@ def assert_foreign_key(table_name)
     assert !result.detect { |row| row["def"] =~ /\AFOREIGN KEY \(.*\) REFERENCES "Users"\("Id"\)\z/ }.nil?, "Missing foreign key on #{table_name}"
   end
 
+  def assert_analyzed(table_pattern, expected = 1)
+    execute("SELECT pg_stat_reset()")
+    yield
+    last_analyzed = execute("SELECT relname, last_analyze FROM pg_stat_user_tables WHERE relname LIKE $1", [table_pattern])
+    # https://github.com/postgres/postgres/commit/375aed36ad83f0e021e9bdd3a0034c0c992c66dc
+    if server_version_num >= 150000
+      assert_equal expected, last_analyzed.count { |v| v["last_analyze"] }
+    end
+  end
+
   # extended statistics are built on partitioned tables
   # https://github.com/postgres/postgres/commit/20b9fa308ebf7d4a26ac53804fce1c30f781d60c
   # (backported to Postgres 10)
@@ -378,11 +395,19 @@ def server_version_num
     execute("SHOW server_version_num").first["server_version_num"].to_i
   end
 
+  def url
+    @url ||= ENV["PGSLICE_URL"] || "postgres:///pgslice_test"
+  end
+
+  def connection
+    @connection ||= PG::Connection.new(url)
+  end
+
   def execute(query, params = [])
     if params.any?
-      $conn.exec_params(query, params)
+      connection.exec_params(query, params)
     else
-      $conn.exec(query)
+      connection.exec(query)
     end
   end
 

test/test_helper.rb

@@ -3,7 +3,4 @@
 require "minitest/autorun"
 require "minitest/pride"
 
-ENV["PGSLICE_ENV"] = "test"
-
-$url = ENV["PGSLICE_URL"] || "postgres:///pgslice_test"
-$conn = PG::Connection.new($url)
+PgSlice::CLI.exit_on_failure = false