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

Handle natively partitioned tables #134

Closed
keithf4 opened this issue Feb 1, 2017 · 20 comments
Closed

Handle natively partitioned tables #134

keithf4 opened this issue Feb 1, 2017 · 20 comments

Comments

@keithf4
Copy link

keithf4 commented Feb 1, 2017

Doing some development work on pg_partman for native partitioning and was confused for a while why some tests were failing. Then realized they only began to fail once I natively partitioned a table. Specifically, I found that has_table fails since it is only looking for a relkind of 'r' in pg_class. Was going to suggest checking for both r and P (new partitioned relkind), but appears that the _rexists() function only accepts a single relkind argument. So either need a whole new has_ function for partitioned tables, or maybe some way to accept multiple relkinds when checking for existence?

keith@keith=# \sf has_table(name, name, text)
CREATE OR REPLACE FUNCTION public.has_table(name, name, text)
 RETURNS text
 LANGUAGE sql
AS $function$
    SELECT ok( _rexists( 'r', $1, $2 ), $3 );
$function$

keith@keith=# \sf _rexists(char, name, name)
CREATE OR REPLACE FUNCTION public._rexists(character, name, name)
 RETURNS boolean
 LANGUAGE sql
AS $function$
    SELECT EXISTS(
        SELECT true
          FROM pg_catalog.pg_namespace n
          JOIN pg_catalog.pg_class c ON n.oid = c.relnamespace
         WHERE c.relkind = $1
           AND n.nspname = $2
           AND c.relname = $3
    );
$function$

@theory
Copy link
Owner

theory commented Feb 1, 2017

I think add partition_exists() and friends.

@keithf4
Copy link
Author

keithf4 commented Feb 1, 2017

Created the following function for myself in order to be able to keep testing for now. Seems to work. However, maybe has_partition() instead to keep consistent with naming?

create or replace function has_partition(name, name, text)
returns text
language sql
as $$
SELECT ok(_rexists('P', $1, $2), $3);
$$;

@theory
Copy link
Owner

theory commented Feb 4, 2017

Yeah, I would expect that to work. I'd just duplicate all these functions, replacing "table" with "partition":

  • has_table()
  • hasnt_table()
  • tables_are()
  • table_owner_is()

Those are the functions I can see right now that are specific to relkind r.

@decibel
Copy link
Collaborator

decibel commented Feb 9, 2017

Except shouldn't any partition test functions accept the parent table as well? partitions_are() is the most obvious example of why you'd want that, but I don't think any of these functions make sense if they ignore the table that a partition belongs to.

For partition_owner_is(), can partitions even have different owners?

@theory
Copy link
Owner

theory commented Feb 14, 2017

Well, should has_table() and friends work on partitions? Or on foreign tables, for that matter?

I don't know how the new partition stuff works (is there new partition support now?). Can/should a parent table have data in it? I mean the root table, of course (assuming the old inheritance partition scheme). To me, that's not a partition, so the partition test functions should ignore it. But maybe things are different in 10.0?

Maybe we should also have an is_partitioned() function for the root table?

@keithf4
Copy link
Author

keithf4 commented Mar 20, 2017

FYI, the functions for this should now be looking for a lower case "p" instead of upper case.

create or replace function has_partition(name, name, text)
returns text
language sql
as $$
SELECT ok(_rexists('p', $1, $2), $3);
$$;

@theory
Copy link
Owner

theory commented Nov 5, 2017

Where are we on this feature?

@keithf4
Copy link
Author

keithf4 commented Nov 5, 2017

EDIT: Sorry, I had my thinking backwards. My has_partition() already checks if a table is a parent. So maybe some extra functions to check if a child has a parent table and/or is a child partition itself?

I've add the two functions for checking if a parent table is partitioned to the README for my testing and been using it without issue

https://github.com/keithf4/pg_partman/blob/master/test/README_test.md#partitioning-in-postgresql-10

@keithf4
Copy link
Author

keithf4 commented Nov 5, 2017

I would also expect the existing functions (has_table(), etc) to work on partition tables. They are still technically tables, just with some extra features the new queries can test for.

@theory
Copy link
Owner

theory commented Nov 5, 2017

If I just copy the materialized-view test to make partition_owner_is(), partitions_are(), and has_partition(), would that be sufficient? If so, is the wording right? has_partition() doesn't test that a partition exists, but that a table is partitioned, right?

@theory
Copy link
Owner

theory commented Nov 5, 2017

Hrm, I'm thinking those ought to be partitioned_owner_is(), partitioneds_are(), and has_partitioned(). Or, no, we just need an is_partitioned() for a parent table, and is_partition() for a child table, no?

@keithf4
Copy link
Author

keithf4 commented Nov 5, 2017

Yes, for has_partition() you feed it the parent table of the partition set to see if it exists and is a partitioned table. But, maybe is[nt]_partitioned() would be a better name? Since has_table() is usually used to check for table existence and should work on partitioned tables as well I think. I'll leave the naming up to you, since I'm fine either way.

Assuming that partitions_are() would take as arguments (parent_schemaname, parent_tablename, [child_table1, child_table2, etc], desc), I think that would work nicely.

@keithf4
Copy link
Author

keithf4 commented Nov 5, 2017

Yeah, I think I like is[nt]_partitioned() and is[nt]_partition() better. Maybe make the latter is[nt]_child_partition() to make the difference in name/function a little clearer? Also possibly have it accept a parent table name to confirm it is the child of a specific parent and not just any child.

@theory
Copy link
Owner

theory commented Nov 5, 2017

How about:

  • *_table* stuff recognizes partitioned tables
  • Add is[nt]_partitioned() to test if a table is partitioned or not
  • Add is[nt]_partition_of() to test that a table is a partition of another table or not (requires the parent table name as a param)
  • Add partitions_are() to test all the partitions of another table (requires the parent table name as a param, as you describe).

@keithf4
Copy link
Author

keithf4 commented Nov 5, 2017

Looks good!

@theory
Copy link
Owner

theory commented Nov 6, 2017

Hrm. Do you know how to tell which table the parent of a partition table? I've not spotted anything useful in the catalogs, but I must be missing it…

@theory
Copy link
Owner

theory commented Nov 6, 2017

Ah, found it in pg_inherits.

@theory theory closed this as completed in 87c3a3d Nov 6, 2017
@decibel
Copy link
Collaborator

decibel commented Nov 6, 2017

Something important to remember: a "child" table can inherit from multiple parents. It would be best if everything that accepts parent also has an array version. For simplicity, probably best if that version just accepts regclass[] instead of schemas name[], parents name[].

It'd also be good if partitions_are() had a version that took regclass[].

@theory
Copy link
Owner

theory commented Nov 6, 2017

Is that true for partitions?

@decibel
Copy link
Collaborator

decibel commented Nov 7, 2017

Ahh, I didn't realize this was strictly testing the declarative partitioning.

I have a feeling this is going to trip some people up, though hopefully the fact that it's only 10+ will limit that.

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