Re: [PROPOSAL] Move all am-related reloption code into src/backend/access/[am-name] and get rid of relopt_kind

From: Nikolay Shaplov <n(dot)shaplov(at)postgrespro(dot)ru>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PROPOSAL] Move all am-related reloption code into src/backend/access/[am-name] and get rid of relopt_kind
Date: 2016-05-27 13:41:41
Message-ID: 1544707.j5kAjF95Mp@nataraj-amd64
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

В письме от 24 мая 2016 17:12:16 пользователь Nikolay Shaplov написал:

While working on this patch I met some difficulties that makes me to completely
rewrite a code that is responsible for interacting reloptions.c with access
methods.

Story start from the point that I found out that a.m. can not forbid changing
some of it's reloptions with ALTER INDEX command. That was not necessary
before, because all reloptions at that existed at that time can be changed on
fly. But now for bloom index it is unacceptable, because for changing bloom's
reloptions for existing index will lead to index malfunction.

I was thinking about adding for_alter flag argument for parseRelOptions funtion
and pass it all the way from indexcmds.c & tablecmds.c, and add some flag in
the definition of reloptions to mark those reloptions that should not be used
in ALTER INDEX clause.

This theoretically this will give us a way to throw error when user tries to
change an reloption that should not be changed.

But unfortunately it turned out that in ATExecSetRelOptions new reloptions is
merged with existing reloptions values using transformRelOptions, and only
after that the result is sent for validation.

So on the step of the validation we can not distinguish old options from a new
one.

I've been thinking how to work this around, but found out that there is no
simple way. I can pass another array of flags through the whole call stack. But
this make all thing uglier.

I can create another function that do pre-validation for new reloptions,
before doing final validation of the whole set. But this will make me to have
to entities available from the a.m.: the descriptor of reloptions and
function for custom validation of the results (i.e. like adjustBloomOptions
for Bloom).

But it would be not good if we dedicate two entires of a.m. to reoptions
definition. And there can be even more...

So I came to a conclusion: gather all the staff that defines the all the
behavior of reloption parsing and validation into one structure, both array of
relopt_value, custom validation function, and all the other staff.

And this structure is the only thing that a.m. gives to reloptions.c code for
parsing and validation purposes. And that should be enough.

I hope this make code more flexible and more easy to understand.

--
Nikolay Shaplov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2016-05-27 13:53:56 Re: COMMENT ON, psql and access methods
Previous Message Michael Paquier 2016-05-27 12:03:14 Re: PATCH: pg_restore parallel-execution-deadlock issue