On Mon, Apr 22, 2019 at 4:43 PM Alvaro Herrera <alvhe...@2ndquadrant.com> wrote: > Well, frequently when people discuss ideas on this list, others discuss > and provide further ideas to try help to find a working solution, rather > than throw every roadblock they can think of (though roadblocks are > indeed thrown now and then). If I've taken a long time to find a > working solution, maybe it's because I have no shoulders of giants to > stand on, and I'm a pretty short guy, so I need to build me a ladder.
What exactly do you mean by throwing up roadblocks? I don't have a basket full of great ideas for how to solve this problem that I'm failing to suggest out of some sort of perverse desire to see you fail. I'm not quite as convinced as Tom and Andres that this whole idea is fatally flawed and can't ever be made to work correctly, but I think it's quite possible that they are right, both because their objections sound to me like they are target and because they are pretty smart people. But that's also because I haven't spent a lot of time on this issue, which I think is pretty fair, because it seems like it would be unfair to complain that I am not spending enough time helping fix code that I advised against committing in the first place. How much time should I spent giving you advice if my previous advice was ignored? But FWIW, it seems to me that a good place to start solving this problem would be to think hard about what Andres said here: http://postgr.es/m/20190306161744.22jdkg37fyi2z...@alap3.anarazel.de Specifically, this complaint: "I still think the feature as is doesn't seem to have very well defined behaviour." If we know what the feature is supposed to do, then it should be possible to look at each relevant piece of code and decides whether it implements the specification correctly or not. But if we don't have a specification -- that is, we don't know precisely what the feature is supposed to do -- then we'll just end up whacking the behavior around trying to get some combination that makes sense, and the whole effort is probably doomed. I think that the large quote block from David Rowley in the middle of the above-linked email gets at the definitional problem pretty clearly: the documentation seems to be intending to say -- although it is not 100% clear -- that if TABLESPACE is specified it has effect on all future children, and if not specified then those children get the tablespace they would have gotten anyway. But that behavior is impossible to implement correctly unless there is a way to distinguish between a partitioned table for which TABLESPACE was not specified and where it was specified to be the same as the default tablespace for the database. And we know, per previous conversation, that the catalog representation admits of no way to make that distinction. Using reltablespace = dattablespace is clearly the wrong answer, because that breaks stuff. Tom earlier suggested, I believe, that some fixed OID could be used, e.g. reltablespace = 1 means "tablespace not specified" and reltablespace = 0 means dattablespace. That should be safe enough, since I don't think OID 1 can ever be the OID of a real tablespace. I think this is probably the only way forward if this definition captures the desired behavior. The other option is to decide that we want some other behavior. In that case, the way forward depends on what we want the behavior to be. Your proposal upthread is to disallow the case where the user provides an explicit TABLESPACE setting whose value matches the default tablespace for the database. But that definition seems to have an obvious problem: just because that's not true when the partitioned table is defined doesn't mean it won't become true later, because the default tablespace for the database can be changed, or the database can be copied and the copy be assigned a different default tablespace. Even if there were no issue, that definition doesn't sound very clean, because it means that reltablespace = 0 for a regular relation means dattablespace and for a partitioned relation it means none. Now that's not the only way we could go either, I suppose. There must be a variety of other possible behaviors. But the one Tom and Andres are proposing -- go back to the way it worked in v10 -- is definitely not crazy. You are right that a lot of people didn't like that, but the definition was absolutely clear, because it was the exact same definition we use for normal tables, with the straigthforward extension that for relations with no storage it meant nothing. Going back to the proposal of making OID = 0 mean TABLESPACE dattablespace, OID = 1 meaning no TABLESPACE clause specified for this partitioned table, and OID = whatever meaning that particular non-default tablespace, I do see a couple of problems with that idea too: - I don't have any idea how to salvage things in v11, where the catalog representation is irredeemably ambiguous. We'd probably have to be satisfied with fairly goofy behavior in v11. - It's not enough to have a good catalog representation. You also have to be able to recreate those catalog states. I think you probably need a way to set the reltablespace to whatever value you need it to have without changing the tables under it. Like ALTER TABLE ONLY blah {TABLESPACE some_tablespace_name | NO TABLESPACE} or some such thing. That exact idea may be wrong, but I think we are not going to have much luck getting to a happy place without something of this sort. If we have a clear definition of what the feature does, a catalog representation to match, and syntax that can recreate any given catalog representation, then this should be an SMOP. But again, rip it all out and try it again someday is not a crazy proposal, IMHO. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
