Thread: ALTER INDEX ... ALTER COLUMN not present in dump
Hello,
It seems like ALTER INDEX ... ALTER COLUMN statements (for setting specific statistics targets on functional indexes, for example) are not part of a pg_dump.
It is not easily noticed, since everything seems to work normally until a sub-par plan is chosen because of an error in cardinality estimates.
Regards,
--
Ronan Dunklau
Hello Can you give reproducible example? I have ALTER TABLE ONLY (schema).(table) ALTER COLUMN (column) SET STATISTICS (target); in pg_dump output. regards, Sergei
> > Hello > Can you give reproducible example? > I have ALTER TABLE ONLY (schema).(table) ALTER COLUMN (column) SET STATISTICS (target); in pg_dump output. > > regards, Sergei Please note it is about ALTER INDEX, not ALTER TABLE. Here is the reproducible example: create table t1 (id int); create index on t1 ((id + 2)); alter index t1 alter column t1_expr statistics 10000; pg_dump output extract: -- -- Name: t1_expr_idx; Type: INDEX; Schema: public; Owner: postgres -- CREATE INDEX t1_expr_idx ON public.t1 USING btree (((id + 2))); -- -- PostgreSQL database dump complete --
Oh, i see. I can reproduce and did not found any SET STATISTICS code for indexes in pg_dump source. Seems completely missed supportfor this clause. regards, Sergei
On 11/15/18 12:20 PM, Sergei Kornilov wrote: > Oh, i see. > I can reproduce and did not found any SET STATISTICS code for indexes in pg_dump source. Seems completely missed supportfor this clause. > > regards, Sergei > It seems we missed something in : https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=5b6d13eec72b960eb0f78542199380e49c8583d4;hp=e09db94c0a5f3b440d96c5c9e8e6c1638d1ec39f Sorry :/
On Thu, Nov 15, 2018 at 12:46:08PM +0100, Adrien NAYRAT wrote: > On 11/15/18 12:20 PM, Sergei Kornilov wrote: >> I can reproduce and did not found any SET STATISTICS code for indexes >> in pg_dump source. Seems completely missed support for this clause. > > It seems we missed something in : > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=5b6d13eec72b960eb0f78542199380e49c8583d4;hp=e09db94c0a5f3b440d96c5c9e8e6c1638d1ec39f Yes, that's a bug, and something that we should try to fix in v11. -- Michael
Attachment
On Fri, Nov 16, 2018 at 07:46:01AM +0900, Michael Paquier wrote: > Yes, that's a bug, and something that we should try to fix in v11. Okay, here are my notes. We need to do a couple of things here: 1) Add a new join to pg_attribute in getIndexes(), then add the information for statistics and the associated column to IndxInfo after parsing the gathered array using parsePGArray(). 2) Add the extra ALTER INDEX commands to the queries creating the objects in dumpIndex(). 3) Add a test. A good thing is that when ALTER INDEX .. SET STATISTICS is applied on an index of a partitioned table, the statement is not cascaded to the existing partitions. We may want in the future to support ONLY and make the inheritance automatic. But that's another topic, and the fix for v11 should be chirurgical. -- Michael
Attachment
On Fri, Nov 16, 2018 at 09:45:05AM +0900, Michael Paquier wrote: > A good thing is that when ALTER INDEX .. SET STATISTICS is applied on an > index of a partitioned table, the statement is not cascaded to the > existing partitions. We may want in the future to support ONLY and make > the inheritance automatic. But that's another topic, and the fix for > v11 should be chirurgical. And here you go as attached. Looking closer, in v10 and older versions, ALTER INDEX SET STATISTICS is able to work as it is an alias of ALTER TABLE. The attached patch does not bother generating the ALTER INDEX queries for v10 and older and feeds queries with empty strings. Perhaps we should support that case? Or the lack of complains would be an argument sufficient to care only about v11 and newer versions? I would tend to think that supporting only v11 and above is enough. Thoughts are welcome. -- Michael
Attachment
On Fri, Nov 16, 2018 at 05:32:52PM +0900, Michael Paquier wrote: > And here you go as attached. Looking closer, in v10 and older versions, > ALTER INDEX SET STATISTICS is able to work as it is an alias of ALTER > TABLE. The attached patch does not bother generating the ALTER INDEX > queries for v10 and older and feeds queries with empty strings. Perhaps > we should support that case? Or the lack of complains would be an > argument sufficient to care only about v11 and newer versions? I would > tend to think that supporting only v11 and above is enough. Thoughts > are welcome. + appendPQExpBuffer(q, "ALTER COLUMN %s ", + indstatcolsarray[j]) This forgot a wrapping with fmtId(). Friday hits hard.. -- Michael
Attachment
On Fri, Nov 16, 2018 at 10:31:03PM +0900, Michael Paquier wrote: > On Fri, Nov 16, 2018 at 05:32:52PM +0900, Michael Paquier wrote: >> And here you go as attached. Looking closer, in v10 and older versions, >> ALTER INDEX SET STATISTICS is able to work as it is an alias of ALTER >> TABLE. The attached patch does not bother generating the ALTER INDEX >> queries for v10 and older and feeds queries with empty strings. Perhaps >> we should support that case? Or the lack of complains would be an >> argument sufficient to care only about v11 and newer versions? I would >> tend to think that supporting only v11 and above is enough. Thoughts >> are welcome. So, any thoughts about this patch? I would still like to move on with only supporting this set of queries only for v11 and above as that gets only clearly documented on the ALTER INDEX page from that point. -- Michael
Attachment
On 11/21/18 1:04 AM, Michael Paquier wrote: > On Fri, Nov 16, 2018 at 10:31:03PM +0900, Michael Paquier wrote: >> On Fri, Nov 16, 2018 at 05:32:52PM +0900, Michael Paquier wrote: >>> And here you go as attached. Looking closer, in v10 and older versions, >>> ALTER INDEX SET STATISTICS is able to work as it is an alias of ALTER >>> TABLE. The attached patch does not bother generating the ALTER INDEX >>> queries for v10 and older and feeds queries with empty strings. Perhaps >>> we should support that case? Or the lack of complains would be an >>> argument sufficient to care only about v11 and newer versions? I would >>> tend to think that supporting only v11 and above is enough. Thoughts >>> are welcome. > > So, any thoughts about this patch? I would still like to move on with > only supporting this set of queries only for v11 and above as that gets > only clearly documented on the ALTER INDEX page from that point. > -- > Michael > Sorry, I will try to look at this soon, but it is relatively new for me. At least, someone else with more knowledge should also look. Thanks to address this issue.
Attachment
On 11/21/18 9:09 AM, Adrien Nayrat wrote: >> So, any thoughts about this patch? I would still like to move on with >> only supporting this set of queries only for v11 and above as that gets >> only clearly documented on the ALTER INDEX page from that point. >> -- >> Michael >> > > Sorry, I will try to look at this soon, but it is relatively new for me. > At least, someone else with more knowledge should also look. > > Thanks to address this issue. > I done some tests and it look good to me. I took an eye on the code and nothing hurt me, but I am not the most qualified to say that. Thanks Michael for the fix!
On Wed, Nov 21, 2018 at 07:24:18PM +0100, Adrien NAYRAT wrote: > I done some tests and it look good to me. I took an eye on the code and > nothing hurt me, but I am not the most qualified to say that. Thanks Adrien for the review. For now I have added it to the next commit fest: https://commitfest.postgresql.org/21/1884/ -- Michael