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

fix: actual table capitalization support #436

Merged
merged 1 commit into from
Apr 1, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
18 changes: 2 additions & 16 deletions pgbelt/cmd/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,7 @@ async def _setup_src_node(

pglogical_tables = pkey_tables
if conf.tables:
pglogical_tables = [
t
for t in pkey_tables
if t
in list(
map(str.lower, conf.tables)
) # Postgres returns table names in lowercase (in analyze_table_pkeys)
]
pglogical_tables = [t for t in pkey_tables if t in conf.tables]

# Intentionally throw an error if no tables are found, so that the user can correct their config.
# When reported by a certain user, errors showed when running the status command, but it was ignored,
Expand Down Expand Up @@ -161,14 +154,7 @@ async def setup_back_replication(config_future: Awaitable[DbupgradeConfig]) -> N

pglogical_tables = pkeys
if conf.tables:
pglogical_tables = [
t
for t in pkeys
if t
in list(
map(str.lower, conf.tables)
) # Postgres returns table names in lowercase (in analyze_table_pkeys)
]
pglogical_tables = [t for t in pkeys if t in conf.tables]

await configure_replication_set(
dst_root_pool, pglogical_tables, conf.schema_name, dst_logger
Expand Down
9 changes: 1 addition & 8 deletions pgbelt/cmd/status.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,14 +92,7 @@ async def status(conf_future: Awaitable[DbupgradeConfig]) -> dict[str, str]:
all_tables = pkey_tables + non_pkey_tables
target_tables = all_tables
if conf.tables:
target_tables = [
t
for t in all_tables
if t
in list(
map(str.lower, conf.tables)
) # Postgres gave us lowercase table names in analyze_table_pkeys
]
target_tables = [t for t in all_tables if t in conf.tables]

if not target_tables:
raise ValueError(
Expand Down
18 changes: 2 additions & 16 deletions pgbelt/cmd/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,7 @@ async def dump_tables(
_, tables, _ = await analyze_table_pkeys(src_pool, conf.schema_name, logger)

if conf.tables:
tables = [
t
for t in tables
if t
in list(
map(str.lower, conf.tables)
) # Postgres returns table names in lowercase (in analyze_table_pkeys)
]
tables = [t for t in tables if t in conf.tables]

await dump_source_tables(conf, tables, logger)

Expand Down Expand Up @@ -192,14 +185,7 @@ async def _dump_and_load_all_tables(
) -> None:
_, tables, _ = await analyze_table_pkeys(src_pool, conf.schema_name, src_logger)
if conf.tables:
tables = [
t
for t in tables
if t
in list(
map(str.lower, conf.tables)
) # Postgres returns table names in lowercase (in analyze_table_pkeys)
]
tables = [t for t in tables if t in conf.tables]
await dump_source_tables(conf, tables, src_logger)
await load_dumped_tables(conf, tables, dst_logger)

Expand Down
2 changes: 1 addition & 1 deletion pgbelt/util/pglogical.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ async def grant_pgl(pool: Pool, tables: list[str], schema: str, logger: Logger)
async with pool.acquire() as conn:
async with conn.transaction():
if tables:
tables_with_schema = [f"{schema}.{table}" for table in tables]
tables_with_schema = [f'{schema}."{table}"' for table in tables]
await conn.execute(
f"GRANT ALL ON TABLE {','.join(tables_with_schema)} TO pglogical;"
)
Expand Down
24 changes: 6 additions & 18 deletions pgbelt/util/postgres.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,12 @@ async def compare_data(
has_run = False
for table in set(pkeys):
# If specific table list is defined and the iterated table is not in that list, skip.
# Note that the pkeys tables returned from Postgres are all lowercased, so we need to
# map the passed conf tables to lowercase.
if tables and (table not in list(map(str.lower, tables))):
if tables and (table not in tables):
continue

has_run = True # If this runs, we have at least one table to compare. We will use this flag to throw an error if no tables are found.

full_table_name = f"{schema}.{table}"
full_table_name = f'{schema}."{table}"'

logger.debug(f"Validating table {full_table_name}...")
order_by_pkeys = ",".join(pkeys_dict[table])
Expand Down Expand Up @@ -387,15 +385,9 @@ async def precheck_info(
)

# We filter the table list if the user has specified a list of tables to target.
# Note, from issue #420, the above query will return the table names in lowercase,
# so we need to map the target_tables to lowercase.
if target_tables:

result["tables"] = [
t
for t in result["tables"]
if t["Name"] in list(map(str.lower, target_tables))
]
result["tables"] = [t for t in result["tables"] if t["Name"] in target_tables]

# We will not recapitalize the table names in the result["tables"] list,
# to preserve how Postgres sees those tables in its system catalog. Easy
Expand All @@ -419,14 +411,10 @@ async def precheck_info(
)

# We filter the table list if the user has specified a list of tables to target.
# Note, from issue #420, the above query will return the table names in lowercase,
# so we need to map the target_tables to lowercase.
if target_sequences:

result["sequences"] = [
t
for t in result["sequences"]
if t["Name"] in list(map(str.lower, target_sequences))
t for t in result["sequences"] if t["Name"] in target_sequences
]

# We will not recapitalize the table names in the result["tables"] list,
Expand Down Expand Up @@ -485,7 +473,7 @@ async def get_dataset_size(

query = f"""
SELECT
sum(pg_total_relation_size(schemaname || '.' || tablename)) AS total_relation_size
sum(pg_total_relation_size(schemaname || '."' || tablename || '"')) AS total_relation_size
FROM
pg_tables
WHERE
Expand All @@ -496,7 +484,7 @@ async def get_dataset_size(
# Yes it's a duplicate, but it's a pretty one. Rather let Postgres do this than Python.
pretty_query = f"""
SELECT
pg_size_pretty(sum(pg_total_relation_size(schemaname || '.' || tablename))) AS total_relation_size
pg_size_pretty(sum(pg_total_relation_size(schemaname || '."' || tablename || '"'))) AS total_relation_size
FROM
pg_tables
WHERE
Expand Down
20 changes: 10 additions & 10 deletions tests/integration/files/test_schema_data.sql
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ ALTER TABLE public.fruits OWNER TO owner;
-- Name: UsersCapital; Type: TABLE; Schema: public; Owner: owner
--

CREATE TABLE public.UsersCapital (
CREATE TABLE public."UsersCapital" (
id bigint NOT NULL,
hash_firstname text NOT NULL,
hash_lastname text NOT NULL,
Expand All @@ -23,13 +23,13 @@ CREATE TABLE public.UsersCapital (
);


ALTER TABLE public.UsersCapital OWNER TO owner;
ALTER TABLE public."UsersCapital" OWNER TO owner;

--
-- Name: UsersCapital2; Type: TABLE; Schema: public; Owner: owner
--

CREATE TABLE public.UsersCapital2 (
CREATE TABLE public."UsersCapital2" (
id bigint NOT NULL,
hash_firstname text NOT NULL,
hash_lastname text NOT NULL,
Expand All @@ -38,13 +38,13 @@ CREATE TABLE public.UsersCapital2 (
);


ALTER TABLE public.UsersCapital2 OWNER TO owner;
ALTER TABLE public."UsersCapital2" OWNER TO owner;

--
-- Name: users_idx; Type: INDEX; Schema: public; Owner: owner
--

CREATE INDEX users_idx ON public.UsersCapital (
CREATE INDEX users_idx ON public."UsersCapital" (
hash_firstname,
hash_lastname
);
Expand All @@ -53,7 +53,7 @@ CREATE INDEX users_idx ON public.UsersCapital (
-- Name: users2_idx; Type: INDEX; Schema: public; Owner: owner
--

CREATE INDEX users2_idx ON public.UsersCapital (
CREATE INDEX users2_idx ON public."UsersCapital" (
hash_firstname,
hash_lastname
);
Expand Down Expand Up @@ -99,7 +99,7 @@ INSERT INTO public.fruits (id, name)
-- Data for Name: UsersCapital; Type: TABLE DATA; Schema: public; Owner: owner
--

INSERT INTO public.UsersCapital (id, hash_firstname, hash_lastname, gender)
INSERT INTO public."UsersCapital" (id, hash_firstname, hash_lastname, gender)
VALUES (1, 'garbagefirst', 'garbagelast', 'male'),
(2, 'garbagefirst1', 'garbagelast1', 'female'),
(3, 'sdgarbagefirst', 'dgsadsrbagelast', 'male'),
Expand All @@ -111,7 +111,7 @@ INSERT INTO public.UsersCapital (id, hash_firstname, hash_lastname, gender)
-- Data for Name: Users2; Type: TABLE DATA; Schema: public; Owner: owner
--

INSERT INTO public.UsersCapital2 (id, hash_firstname, hash_lastname, gender)
INSERT INTO public."UsersCapital2" (id, hash_firstname, hash_lastname, gender)
VALUES (1, 'garbagefirst', 'garbagelast', 'male'),
(2, 'garbagefirst1', 'garbagelast1', 'female'),
(3, 'sdgarbagefirst', 'dgsadsrbagelast', 'male'),
Expand All @@ -137,13 +137,13 @@ SELECT pg_catalog.setval('public.users2_id_seq', 1, false);
-- Name: UsersCapital users_pkey; Type: CONSTRAINT; Schema: public; Owner: owner
--

ALTER TABLE ONLY public.UsersCapital
ALTER TABLE ONLY public."UsersCapital"
ADD CONSTRAINT users_pkey PRIMARY KEY (id);


--
-- Name: UsersCapital users_pkey; Type: CONSTRAINT; Schema: public; Owner: owner
--

ALTER TABLE ONLY public.UsersCapital2
ALTER TABLE ONLY public."UsersCapital2"
ADD CONSTRAINT users2_pkey PRIMARY KEY (id);
Loading