On Tue, 2003-08-26 at 03:10, Barry Lind wrote:
> Fernando Nasser wrote:
> > Hi Barry,
> >
> > Barry Lind wrote:
> >
> >> Kim, Fernando,
> >>
> >> I have been studying this patch. I would like to bounce some of my
> >> concerns off of the two of you to see if we (or anyone else on the
> >> list) can come up with a better solution.
> >>
> >> My concerns are:
> >>
> >> 1) There is no way for anyone else who is adding code to the driver to
> >> understand what all these SQL_STATE values mean. For example if I
> >> were to add code that threw a sql exception now, I would have no idea
> >> what (if any) sql state value should be included. To me they are
> >> random five digit strings that have no meaning. To be maintainable
> >> there needs to be a way for all contributors to understand what each
> >> value means and what the list of values the driver uses are.
> >>
> >
> > The SQLState values are defined in the SQL Standard. They are not
> > RANDOM values, they are standard values.
> >
> > Whenever the condition was in the implementation define range we used
> > the "de facto" standard values as assigned by the ODBC (also used by
> > both DB2 and Oracle).
> >
> > All these are public documents.
>
> I should have put random in quotes. I know that they are not truely
> random, but if one doesn't know what the values mean, they appear that
> way. While it may be true that these are all documented somewhere, the
> problem is that I don't know where that is. So at a minimum there needs
> to be comments in the relevant code that point myself (and any others
> who will ever add code to the driver that throws a sql exception) to the
> correct location, so that we can do the right thing when it comes to the
> sql state values.
>
> >
> >> 2) This is related to the first, but I don't like the use of String
> >> values in the constructor to SqlState. They all look the same and
> >> from a code review standpoint, even if I knew what all the values
> >> were, it would be too easy to either mistype or otherwise pass in the
> >> wrong value. In something like this I think some sort of constants
> >> with descriptive names would be much better than the five digit codes
> >> (get some compile time checking).
> >>
> >
> > Good suggestion. Kim is adding constants to the PSQLState class so the
> > 5 character strings will become something like PSQLState.SomeCondition.
> >
>
> I think that will go a long way to address issue one above as well.
I suggest you use the names from the server source code in the file
src/include/utils/errcodes.h, there may be some issue keeping them
synchronized, but this is a better start than creating our own names.
>
> >
> >> 3) There are now too many constructors (IMHO) for PSQLException. It
> >> isn't obvious what constructor does what, and under what circumstances
> >> you would use one or the other. Perhaps the structure of this class
> >> needs some thought.
> >>
> >
> > This is not our doing. This is the current state of the driver. Kim is
> > actually proposing a considerably smaller number of signatures just to
> > improve this situation.
> >
>
> In part it is your doing, since the patch as it stands only makes things
> worse by adding yet more.
>
> >
> >> 4) Related to 3 above, is it necessary to still have the old
> >> constructors for PSQLException that don't take a SqlState object, or
> >> should all PSQLExceptions have a SqlState, even if much of the time it
> >> is some generic standard value?
> >>
> >
> > As Kim and I mentioned in some of the messages we've sent to you, we are
> > leaving the old signatures in place so that the conversion can be done
> > in steps. They will go away after all calls are replaced. We thought
> > this was better than sending a too pervasive mega-patch.
> >
>
> Then the old signatures should be marked deprecated.
>
> I appreciate the thought about not sending a mega-patch, but at the same
> time I don't like seeing things half done. I guess I just want it both
> ways:-). As long as I have your commitment that the rest is coming
> this is fine for now.
>
> Thanks for the work on this feature. This feature is a long standing
> user requested enhancement to the driver. It is good to see that it is
> getting done in 7.4.
>
> thanks,
> --Barry
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Don't 'kill -9' the postmaster
>
--
Dave Cramer <dave(at)fastcrypt(dot)com>
fastcrypt