On Tue, Aug 10, 2021 at 12:24:13PM +0900, Michael Paquier wrote:
> So, on a closer look, it happens that this breaks the regression tests
> of sepgsql, as the two following commands in ddl.sql cause a rewrite:
> ALTER TABLE regtest_table_4 ALTER COLUMN y TYPE float;
> ALTER TABLE regtest_ptable_4 A
On Sat, Aug 07, 2021 at 07:18:19PM +0900, Michael Paquier wrote:
> As a matter of curiosity, I have checked how it would look to handle
> the no-op case for the sub-commands other than SET TABLESPACE, and one
> would need something like the attached, with a new flag for
> AlteredTableInfo. That do
On Fri, Jul 30, 2021 at 02:18:02PM -0700, Jeff Davis wrote:
> It sounds like anything we do here should be part of a larger change to
> make it consistent. So I'm fine with the patch you posted.
As a matter of curiosity, I have checked how it would look to handle
the no-op case for the sub-command
On Fri, 2021-07-30 at 16:22 +0900, Michael Paquier wrote:
> Looking at the past, it was the intention of 05f3f9c7 to go through
> the hook even if SET TABLESPACE does not move the relation, so you
> are
> right that ALTER TABLE is inconsistent to not do the same for LOGGED,
> UNLOGGED and ACCESS ME
On Thu, Jul 29, 2021 at 08:55:21AM -0700, Jeff Davis wrote:
> I see that ATExecSetTableSpace() also invokes the hook even for a no-
> op. Should we do the same thing for setting the AM?
Looking at the past, it was the intention of 05f3f9c7 to go through
the hook even if SET TABLESPACE does not mov
On Thu, 2021-07-29 at 15:27 +0900, Michael Paquier wrote:
> Doing any checks around the hooks of objectaccess.h is very annoying,
> because we have no modules to check after them easily except sepgsql.
> Anyway, I have been checking that, with the hack-ish module attached
> and tracked down that sw
On Wed, Jul 28, 2021 at 01:05:10PM -0700, Jeff Davis wrote:
> On Wed, 2021-07-28 at 14:02 +0900, Michael Paquier wrote:
>> Right. Isn't that an older issue though? A rewrite involved after a
>> change of relpersistence does not call the hook either. It looks to
>> me that this should be after fi
On Wed, 2021-07-28 at 14:02 +0900, Michael Paquier wrote:
> Arg, sorry about that! I was unclear what the situation of the patch
> was.
No problem, race condition ;-)
> Right. Isn't that an older issue though? A rewrite involved after a
> change of relpersistence does not call the hook either.
On Tue, Jul 27, 2021 at 08:40:59PM -0700, Jeff Davis wrote:
> I just returned from vacation and I was about ready to commit this
> myself, but I noticed that it doesn't seem to be calling
> InvokeObjectPostAlterHook().
Arg, sorry about that! I was unclear what the situation of the patch
was.
> I
On Wed, 2021-07-28 at 12:23 +0900, Michael Paquier wrote:
> On Tue, Jul 27, 2021 at 04:38:48PM +0900, Michael Paquier wrote:
> > Okay, hearing nothing, I have looked again at 0001 and did some
> > light
> > adjustments, mainly in the tests. I did not spot any issues in the
> > patch, so that looks
On Tue, Jul 27, 2021 at 04:38:48PM +0900, Michael Paquier wrote:
> Okay, hearing nothing, I have looked again at 0001 and did some light
> adjustments, mainly in the tests. I did not spot any issues in the
> patch, so that looks good to go for me.
And done as of b048326.
--
Michael
signature.as
On Thu, Jul 22, 2021 at 04:41:54AM -0500, Justin Pryzby wrote:
> It looks like one hunk was missing/uncommitted from the 0002 patch..
Okay, hearing nothing, I have looked again at 0001 and did some light
adjustments, mainly in the tests. I did not spot any issues in the
patch, so that looks good
On Thu, Jul 22, 2021 at 10:37:12AM +0530, vignesh C wrote:
> On Fri, Jul 16, 2021 at 9:19 AM Justin Pryzby wrote:
> >
> > rebased.
> >
> > Also, there were two redundant checks for multiple SET ACCESS METHOD
> > commands.
> > But one of them wasn't hit if the ALTER was setting the current AM due
On Fri, Jul 16, 2021 at 9:19 AM Justin Pryzby wrote:
>
> rebased.
>
> Also, there were two redundant checks for multiple SET ACCESS METHOD commands.
> But one of them wasn't hit if the ALTER was setting the current AM due to the
> no-op test.
>
> I think it's better to fail in every case, and not
On Thu, Jul 15, 2021 at 10:49:23PM -0500, Justin Pryzby wrote:
> Also, there were two redundant checks for multiple SET ACCESS METHOD commands.
> But one of them wasn't hit if the ALTER was setting the current AM due to the
> no-op test.
Yep.
> I think it's better to fail in every case, and not j
rebased.
Also, there were two redundant checks for multiple SET ACCESS METHOD commands.
But one of them wasn't hit if the ALTER was setting the current AM due to the
no-op test.
I think it's better to fail in every case, and not just sometimes (especially
if we were to use ERRCODE_SYNTAX_ERROR).
On Thu, Jun 10, 2021 at 1:01 AM Jeff Davis wrote:
>
> On Wed, 2021-06-09 at 13:47 +0900, Michael Paquier wrote:
> > There is a mix of upper and lower-case characters here. It could be
> > more consistent. It seems to me that this test should actually check
> > that pg_class.relam reflects the ne
On Wed, Jun 09, 2021 at 01:45:52PM -0700, Zhihong Yu wrote:
> + /* check if another access method change was already requested
> */
> + if (tab->newAccessMethod)
> + ereport(ERROR,
> + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> +
On Wed, Jun 09, 2021 at 01:47:18PM +0900, Michael Paquier wrote:
> On Tue, Jun 08, 2021 at 05:33:31PM -0700, Jeff Davis wrote:
> > New version attached, with the detoasting code removed. Could use
> > another round of validation/cleanup in case I missed something during
> > the merge.
>
> This loo
On Wed, Jun 09, 2021 at 01:45:52PM -0700, Zhihong Yu wrote:
> + /* check if another access method change was already requested
> */
> + if (tab->newAccessMethod)
> + ereport(ERROR,
> + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> +
On Wed, Jun 9, 2021 at 12:31 PM Jeff Davis wrote:
> On Wed, 2021-06-09 at 13:47 +0900, Michael Paquier wrote:
> > There is a mix of upper and lower-case characters here. It could be
> > more consistent. It seems to me that this test should actually check
> > that pg_class.relam reflects the new
On Wed, 2021-06-09 at 13:47 +0900, Michael Paquier wrote:
> There is a mix of upper and lower-case characters here. It could be
> more consistent. It seems to me that this test should actually check
> that pg_class.relam reflects the new value.
Done. I also added a (negative) test for changing t
On Tue, Jun 08, 2021 at 05:33:31PM -0700, Jeff Davis wrote:
> New version attached, with the detoasting code removed. Could use
> another round of validation/cleanup in case I missed something during
> the merge.
This looks rather sane to me, thanks.
/* Create the transient table that will re
On Thu, 2021-06-03 at 14:36 -0700, Jeff Davis wrote:
> Do we have general agreement on this point? Did I miss another
> purpose
> of detoasting in tablecmds.c, or can we just remove that part of the
> patch?
Per discussion with Justin, I'll drive this patch. I merged the CF
entries into https://co
On Fri, Jun 04, 2021 at 05:34:36PM -0700, Jeff Davis wrote:
> I agree that a dummy AM would be good, but implementing even a dummy AM
> is a fair amount of work.
Not much, honestly, the largest part being to document that properly
so as it could be used as a template:
https://www.postgresql.org/me
On Sat, 2021-06-05 at 08:45 +0900, Michael Paquier wrote:
> One thing I am wondering is if we should have a dummy_table_am in
> src/test/modules/ to be able to stress more this feature. That does
> not seem like a hard requirement, but relying only on heap limits a
> bit the coverage of this featu
On Fri, Jun 04, 2021 at 11:26:28AM -0700, Jeff Davis wrote:
> Yes. That's a current requirement, and any AM that doesn't do that is
> already broken (e.g. for INSERT INTO ... SELECT).
Makes sense. I was just looking at the patch, and this was the only
part of it that made my spidey sense react.
On Fri, 2021-06-04 at 14:58 +0900, Michael Paquier wrote:
> In short, a table AMs would receive on a rewrite with ALTER TABLE
> tuples which may be toasted, still table_insert_tuple() should be
> able
> to handle both:
> - the case where this tuple was already toasted.
> - the case where this tuple
On Thu, Jun 03, 2021 at 02:36:15PM -0700, Jeff Davis wrote:
> Do we have general agreement on this point? Did I miss another purpose
> of detoasting in tablecmds.c, or can we just remove that part of the
> patch?
Catching up with this thread.. So, what you are suggesting here is
that we have no n
On Thu, 2021-05-06 at 17:24 -0700, Jeff Davis wrote:
> It's the table AM's responsibility to detoast out-of-line datums and
> toast any values that are too large (see
> heapam.c:heap_prepare_insert()).
Do we have general agreement on this point? Did I miss another purpose
of detoasting in tablecmd
On Thu, 2021-05-06 at 17:19 -0500, Justin Pryzby wrote:
> If ATRewriteTable didn't
> handle this,
> data would become inaccessible if an AM failed to de-toast tuples.
If the AM fails to detoast tuples, it's got bigger problems than ALTER
TABLE. What about INSERT INTO ... SELECT?
It's the table AM
On Thu, May 06, 2021 at 02:11:31PM -0700, Jeff Davis wrote:
> On Thu, 2021-05-06 at 15:23 -0500, Justin Pryzby wrote:
> > I think ALTER TABLE SET ACCESS METHOD should move all data off the
> > old AM,
> > including its toast table.
>
> Can you explain what you mean, and why? I'm still confused.
>
On Thu, 2021-05-06 at 15:23 -0500, Justin Pryzby wrote:
> I think ALTER TABLE SET ACCESS METHOD should move all data off the
> old AM,
> including its toast table.
Can you explain what you mean, and why? I'm still confused.
Let's say there are 4 table AMs: A, AT, B, and BT. A's
relation_toast_am(
On Thu, May 06, 2021 at 01:10:53PM -0700, Jeff Davis wrote:
> On Mon, 2021-03-08 at 16:30 +0900, Michael Paquier wrote:
> > This toast issue is a kind of interesting one, and it seems rather
> > right to rely on toast_build_flattened_tuple() to decompress things
> > if
> > both table AMs support to
On Mon, 2021-03-08 at 16:30 +0900, Michael Paquier wrote:
> This toast issue is a kind of interesting one, and it seems rather
> right to rely on toast_build_flattened_tuple() to decompress things
> if
> both table AMs support toast with the internals of toast knowing what
> kind of compression has
On Mon, Mar 08, 2021 at 04:30:23PM +0900, Michael Paquier wrote:
> This toast issue is a kind of interesting one, and it seems rather
> right to rely on toast_build_flattened_tuple() to decompress things if
> both table AMs support toast with the internals of toast knowing what
> kind of compressio
On Sun, Mar 07, 2021 at 07:07:07PM -0600, Justin Pryzby wrote:
> This renames to use SET ACCESS METHOD (resolving a silly typo);
> And handles the tuple slots more directly;
> And adds docs and tab completion;
>
> Also, since 8586bf7ed8889f39a59dd99b292014b73be85342:
> |For now it's not allowe
On Mon, Mar 01, 2021 at 11:16:36AM +0900, Michael Paquier wrote:
> On Sun, Feb 28, 2021 at 04:25:30PM -0600, Justin Pryzby wrote:
> > I called this "set TABLE access method" rather than just "set access method"
> > for the reasons given on the LIKE thread:
> > https://www.postgresql.org/message-id/
On Mon, Mar 01, 2021 at 11:16:36AM +0900, Michael Paquier wrote:
> On Sun, Feb 28, 2021 at 04:25:30PM -0600, Justin Pryzby wrote:
> > I called this "set TABLE access method" rather than just "set access method"
> > for the reasons given on the LIKE thread:
> > https://www.postgresql.org/message-id/
On Sun, Feb 28, 2021 at 04:25:30PM -0600, Justin Pryzby wrote:
> I called this "set TABLE access method" rather than just "set access method"
> for the reasons given on the LIKE thread:
> https://www.postgresql.org/message-id/20210119210331.gn8...@telsasoft.com
ALTER TABLE applies to a table (or p
40 matches
Mail list logo
