Thread: Re: [HACKERS] Array bug is still there....
Andrew Martin wrote: > Just run the regression tests on 6.1 and as I suspected the array bug > is still there. The regression test passes because the expected output > has been fixed to the *wrong* output. OK, I think I understand the current array behavior, which is apparently different than the behavior for v1.0x. Postgres v6.1 allows one to specify a dimensionality for an array object when declaring that object/column. However, that specification is not used when decoding a field. Instead, the dimensionality is deduced from the input string itself. The dimensionality is stored with each field, and is used to encode the array on output. So, one is currently allowed to mix array dimensions within a column, but Postgres seems to keep that all straight for input and output. Is this the behavior that we want? Just because it is different from previous behavior doesn't mean that it is undesirable. However, when mixing dimensionality within the same column it must be more difficult to figure out how to do comparison and other operations on arrays. If we are to enforce dimensionality within columns, then I need to figure out how to get that information from the table declaration when decoding fields. Bruce, do you know where to look for that kind of code? Anyone have an idea on how much this code has changed over the last year?? - Tom --ELM913966242-1523-0_ --ELM913966242-1523-0_--
Added to TODO list, and I will save this message if anyone has any questions later. > > > Just run the regression tests on 6.1 and as I suspected the array bug > is still there. The regression test passes because the expected output > has been fixed to the *wrong* output. > > As I have said before, I don't use arrays, so this doesn't affect me, > but it is very bad practice that this should be released with the > regression tests claiming that arrays work when they don't. > > Please make sure that this is in the TODO list and for the next release > it really should be fixed or array functionality disabled. > > > The current (wrong) expected output is: > > QUERY: SELECT * FROM arrtest; > a |b |c |d |e > -----------+---------------+-------------+-----------------+------------- > {1,2,3,4,5}|{{{0,0},{1,2}}}|{} |{} | > {11,12,23} |{{3,4},{4,5}} |{"foobar"} |{{"elt1","elt2"}}|{"3.4","6.7"} > {} |{3,4} |{"foo","bar"}|{"bar","foo"} | > > > The correct output is: > > QUERY: SELECT * FROM arrtest; > a b c d e > ------------ ---------------------- -------------- ------------------ -------------- > {1,2,3,4,5} {{{0,0}},{{1,2}}} {} {} > {11,12,23} {{{3},{4}},{{4},{5}}} {"foobar"} {{"elt1","elt2"}} {"3.4","6.7"} > {} {{{3,4},{0,0}}} {"foo","bar"} {{"bar"},{"foo"}} > > > > The problem comes in columns b and d which are 3D and 2D arrays. > The wrong output is munging the dimensionality of these arrays. > > > Surely the CVS logs can tell us where the bug was introduced. I'm > fairly sure it was somewhere between 1.02 and 1.08. > > > Andrew > > ---------------------------------------------------------------------------- > Dr. Andrew C.R. Martin University College London > EMAIL: (Work) martin@biochem.ucl.ac.uk (Home) andrew@stagleys.demon.co.uk > URL: http://www.biochem.ucl.ac.uk/~martin > Tel: (Work) +44(0)171 419 3890 (Home) +44(0)1372 275775 > > - -- Bruce Momjian maillist@candle.pha.pa.us ------------------------------
> Andrew Martin wrote: > > > > Just run the regression tests on 6.1 and as I suspected the array bug > > is still there. The regression test passes because the expected output > > has been fixed to the *wrong* output. > > > > As I have said before, I don't use arrays, so this doesn't affect me, > > but it is very bad practice that this should be released with the > > regression tests claiming that arrays work when they don't. > > >From the regression test README: > > The interpretation of array specifiers (the curly braces around atomic > values) appears to have changed sometime after the original regression > tests were generated. The current ./expected/*.out files reflect this > new interpretation, which may not be correct! I'm sure it is not correct! In the example below, `b' is defined as a 3-dimensional array. From the old output, you could clearly see that it had 3 dimensions in all rows, now you can't. Obviously that is wrong... > > It seems to me to be *pointless* to carry forward long-term behavior as > a failure for regression testing when the regression testing really is > designed to ensure that behavior is consistant and that a new > installation is functioning "properly", which means that it behaves as > well as a reference installation behaves. I believe that is a minor role for regression tests. Their primary role is for *developers* to check that their changes haven't broken anything. Installers/users shouldn't really need them other than as a check that they haven't done anything completely stupid during the installation. Incidently, the new style regression tests are excellent. *Much much* better than the old ones :-) > Don't know who uses arrays, > but it apparently isn't anyone who cares enough to try fixing the > problem. I agree. I've never used them and don't know anyone who has. My point is that if this is a feature we are keeping then it should work properly. If not, then it could be dumped like time travel, but I'm sure there must be some people out there somewhere who do use it :-) > > (OK, I feel better now :) > > Do you actually understand how the array output is supposed to look? Yes! Look in the old regression test expected output... > Since no versions of the code do it "correctly", I've been confused as > to what the array rules really are. Huh? It certainly used to work in the Andrew & Jolly days and I'm sure it worked for a while while under the net-development team. As I said earlier, it broke somewhere between 1.02 and 1.08 > Is the stuff munged internally, or > only for output?? Wouldn't mind working on it but need to understand the > correct behavior first... I don't know where it got broken, but it must be traceable from the CVS logs... Best wishes, Andrew - ---------------------------------------------------------------------------- Dr. Andrew C.R. Martin University College London EMAIL: (Work) martin@biochem.ucl.ac.uk (Home) andrew@stagleys.demon.co.uk URL: http://www.biochem.ucl.ac.uk/~martin Tel: (Work) +44(0)171 419 3890 (Home) +44(0)1372 275775 ------------------------------
Just run the regression tests on 6.1 and as I suspected the array bug is still there. The regression test passes because the expected output has been fixed to the *wrong* output. As I have said before, I don't use arrays, so this doesn't affect me, but it is very bad practice that this should be released with the regression tests claiming that arrays work when they don't. Please make sure that this is in the TODO list and for the next release it really should be fixed or array functionality disabled. The current (wrong) expected output is: QUERY: SELECT * FROM arrtest; a |b |c |d |e - -----------+---------------+-------------+-----------------+------------- {1,2,3,4,5}|{{{0,0},{1,2}}}|{} |{} | {11,12,23} |{{3,4},{4,5}} |{"foobar"} |{{"elt1","elt2"}}|{"3.4","6.7"} {} |{3,4} |{"foo","bar"}|{"bar","foo"} | The correct output is: QUERY: SELECT * FROM arrtest; a b c d e - ------------ ---------------------- -------------- ------------------ -------------- {1,2,3,4,5} {{{0,0}},{{1,2}}} {} {} {11,12,23} {{{3},{4}},{{4},{5}}} {"foobar"} {{"elt1","elt2"}} {"3.4","6.7"} {} {{{3,4},{0,0}}} {"foo","bar"} {{"bar"},{"foo"}} The problem comes in columns b and d which are 3D and 2D arrays. The wrong output is munging the dimensionality of these arrays. Surely the CVS logs can tell us where the bug was introduced. I'm fairly sure it was somewhere between 1.02 and 1.08. Andrew - ---------------------------------------------------------------------------- Dr. Andrew C.R. Martin University College London EMAIL: (Work) martin@biochem.ucl.ac.uk (Home) andrew@stagleys.demon.co.uk URL: http://www.biochem.ucl.ac.uk/~martin Tel: (Work) +44(0)171 419 3890 (Home) +44(0)1372 275775 ------------------------------
Andrew Martin wrote: > > Just run the regression tests on 6.1 and as I suspected the array bug > is still there. The regression test passes because the expected output > has been fixed to the *wrong* output. > > As I have said before, I don't use arrays, so this doesn't affect me, > but it is very bad practice that this should be released with the > regression tests claiming that arrays work when they don't. from the regression test README: The interpretation of array specifiers (the curly braces around atomic values) appears to have changed sometime after the original regression tests were generated. The current ./expected/*.out files reflect this new interpretation, which may not be correct! It seems to me to be *pointless* to carry forward long-term behavior as a failure for regression testing when the regression testing really is designed to ensure that behavior is consistant and that a new installation is functioning "properly", which means that it behaves as well as a reference installation behaves. Don't know who uses arrays, but it apparently isn't anyone who cares enough to try fixing the problem. (OK, I feel better now :) Do you actually understand how the array output is supposed to look? Since no versions of the code do it "correctly", I've been confused as to what the array rules really are. Is the stuff munged internally, or only for output?? Wouldn't mind working on it but need to understand the correct behavior first... - Tom > > Please make sure that this is in the TODO list and for the next release > it really should be fixed or array functionality disabled. > > The current (wrong) expected output is: > > QUERY: SELECT * FROM arrtest; > a |b |c |d |e > -----------+---------------+-------------+-----------------+------------- > {1,2,3,4,5}|{{{0,0},{1,2}}}|{} |{} | > {11,12,23} |{{3,4},{4,5}} |{"foobar"} |{{"elt1","elt2"}}|{"3.4","6.7"} > {} |{3,4} |{"foo","bar"}|{"bar","foo"} | > > The correct output is: > > QUERY: SELECT * FROM arrtest; > a b c d e > ------------ ---------------------- -------------- ------------------ -------------- > {1,2,3,4,5} {{{0,0}},{{1,2}}} {} {} > {11,12,23} {{{3},{4}},{{4},{5}}} {"foobar"} {{"elt1","elt2"}} {"3.4","6.7"} > {} {{{3,4},{0,0}}} {"foo","bar"} {{"bar"},{"foo"}} > > The problem comes in columns b and d which are 3D and 2D arrays. > The wrong output is munging the dimensionality of these arrays. ------------------------------
Andrew Martin wrote: > Just run the regression tests on 6.1 and as I suspected the array bug > is still there. The regression test passes because the expected output > has been fixed to the *wrong* output. OK, I think I understand the current array behavior, which is apparently different than the behavior for v1.0x. Postgres v6.1 allows one to specify a dimensionality for an array object when declaring that object/column. However, that specification is not used when decoding a field. Instead, the dimensionality is deduced from the input string itself. The dimensionality is stored with each field, and is used to encode the array on output. So, one is currently allowed to mix array dimensions within a column, but Postgres seems to keep that all straight for input and output. Is this the behavior that we want? Just because it is different from previous behavior doesn't mean that it is undesirable. However, when mixing dimensionality within the same column it must be more difficult to figure out how to do comparison and other operations on arrays. If we are to enforce dimensionality within columns, then I need to figure out how to get that information from the table declaration when decoding fields. Bruce, do you know where to look for that kind of code? Anyone have an idea on how much this code has changed over the last year?? - Tom ------------------------------
> > On Thu, 19 Jun 1997, Andrew Martin wrote: > > > > Don't know who uses arrays, > > > but it apparently isn't anyone who cares enough to try fixing the > > > problem. > > > > I agree. I've never used them and don't know anyone who has. My point > > is that if this is a feature we are keeping then it should work > > properly. If not, then it could be dumped like time travel, but I'm > > sure there must be some people out there somewhere who do use it :-) > > I use arrays. I wrote a time sheet program. For a given employee, > month and contract I store, among other things, an array giving the > hours worked for each day of the month. > > David Friend ! cq995@freenet.carleton.ca > Atlantis Scientific Inc. ! david.friend@atlsci.com > 20 Colonnade Rd, Suite 110 ! 613-727-1087 (voice) > Ottawa, Ontario, CANADA K2E 7M6 ! 800-265-3894 (voice) > ERGOvista Scientific Image Analysis ! 613-727-5853 (fax) > But do you use *multi-dimensional* arrays (that's really what I should have said rather than just arrays). 1D arrays seem to work fine. It's the multi-D arrays where the output makes no sense. Andrew - ---------------------------------------------------------------------------- Dr. Andrew C.R. Martin University College London EMAIL: (Work) martin@biochem.ucl.ac.uk (Home) andrew@stagleys.demon.co.uk URL: http://www.biochem.ucl.ac.uk/~martin Tel: (Work) +44(0)171 419 3890 (Home) +44(0)1372 275775 ------------------------------
On Thu, 19 Jun 1997, Andrew Martin wrote: > > Don't know who uses arrays, > > but it apparently isn't anyone who cares enough to try fixing the > > problem. > > I agree. I've never used them and don't know anyone who has. My point > is that if this is a feature we are keeping then it should work > properly. If not, then it could be dumped like time travel, but I'm > sure there must be some people out there somewhere who do use it :-) I use arrays. I wrote a time sheet program. For a given employee, month and contract I store, among other things, an array giving the hours worked for each day of the month. David Friend ! cq995@freenet.carleton.ca Atlantis Scientific Inc. ! david.friend@atlsci.com 20 Colonnade Rd, Suite 110 ! 613-727-1087 (voice) Ottawa, Ontario, CANADA K2E 7M6 ! 800-265-3894 (voice) ERGOvista Scientific Image Analysis ! 613-727-5853 (fax) ------------------------------
dfriend@atlsci.atlsci.com said: :- I use arrays. Me too. All over. Very important to me. - ----------------------------------------------------------------------------- Robert Withrow, Tel: +1 617 592 8935, Net: witr@rwwa.COM ------------------------------
At 8:16 AM 6/24/97, Thomas G. Lockhart wrote: >Postgres v6.1 allows one to specify a dimensionality for an array object >when declaring that object/column. However, that specification is not >used when decoding a field. Instead, the dimensionality is deduced from >the input string itself. The dimensionality is stored with each field, >and is used to encode the array on output. So, one is currently allowed >to mix array dimensions within a column, but Postgres seems to keep that >all straight for input and output. This sounds funny to me. You mean if a column contains an array, its dimension could vary from row to row? Maybe you want that sometimes, but I wouldn't think that was normally desirable. Maybe the correct solution is to have a normal, fixed dimension array type and a variable-dimension array type as well. If we have to pick one of the two I would vote for the fixed-dimension one. I think that is what you would normally want and it should find some kinds of user errors that the variable-dimension array type would miss. Signature failed Preliminary Design Review. Feasibility of a new signature is currently being evaluated. h.b.hotz@jpl.nasa.gov, or hbhotz@oxy.edu ------------------------------
> At 8:16 AM 6/24/97, Thomas G. Lockhart wrote: > >Postgres v6.1 allows one to specify a dimensionality for an array object > >when declaring that object/column. However, that specification is not > >used when decoding a field. Instead, the dimensionality is deduced from > >the input string itself. The dimensionality is stored with each field, > >and is used to encode the array on output. So, one is currently allowed > >to mix array dimensions within a column, but Postgres seems to keep that > >all straight for input and output. > > This sounds funny to me. You mean if a column contains an array, its > dimension could vary from row to row? Maybe you want that sometimes, but I > wouldn't think that was normally desirable. > > Maybe the correct solution is to have a normal, fixed dimension array type > and a variable-dimension array type as well. If we have to pick one of the > two I would vote for the fixed-dimension one. I think that is what you > would normally want and it should find some kinds of user errors that the > variable-dimension array type would miss. > The current behaviour isn't really a variable dimension array since what you put in is not what you get out. You put in lots of curly brackets to indicate the dimensionality, but you get out something which loses all this information. Andrew - ---------------------------------------------------------------------------- Dr. Andrew C.R. Martin University College London EMAIL: (Work) martin@biochem.ucl.ac.uk (Home) andrew@stagleys.demon.co.uk URL: http://www.biochem.ucl.ac.uk/~martin Tel: (Work) +44(0)171 419 3890 (Home) +44(0)1372 275775 ------------------------------