On Wed, Dec 05, 2001 at 02:32:46PM -0500, Jeff Urlwin wrote:
> I've looked at the patch. Please see my comments below.
>
> >
> > On Tue, Dec 04, 2001 at 08:00:15PM +0100, Flemming Frandsen wrote:
> > > Tim Bunce wrote:
> > > >
> > > > I'd rather not go that way (I don't like solutions that require
> > > > mangling the sql in odd ways).
> > >
> > > Hmm, ok then.
> > >
> > >
> > > > I think I'd rather optionally ignore placeholders named
> > ':old' and ':new'.
> > > > Also, I think there was a patch floating around that optionally
> > > > disabled :foo style placeholders. That would also suffice.
> > >
> > > Ok, here it is, it ignores and passes through stuff that starts out like
> > > a : placeholder and ends with a ., as far as I know there is no use for
> > > a dot right after a placeholder so this ought to be 100% safe and
> > > transparent.
> >
> > I was going to suggest that originally but I suspect there is valid
> > trigger syntax that uses :old or :new without a trailing .fieldname.
> > (I'm pretty sure there is for Oracle.)
>
> Yes, I believe there is, especially if you have a stored proc and are
> passing a ROWTYPE. I'd like to do this either:
> 1) :abc.fieldname gets ignored, as per the patch and document the caveat
> above.
> or
> 2) \:foo means ignore it as a parameter
> or
> 3) Add an attribute to Ingore :foo parameters (but not :/\d+/ or ?). That
> would be an ODBC private attribute??? or a DBI one???
>
> The downside of number two is the same as the :: problem, except it has the
> benefit of being relatively standard, in terms of escape
> sequences...however, the triggers get rewritten, which can be painful when
> cutting and pasting from a SQL tool such as TOAD...
>
> I actually prefer #3, honestly.
Me too.
> The only question I have is, should this be
> a DBI attribute? It seems that since DBD::ODBC is parsing this and not
> letting DBI do the work, I still have to do the work and honor the
> attribute. Is there a chance that this is handy across all DBI?
Make it a driver-private attribute for now. Maybe
odbc_ignore_named_placeholders => 1
Umm, or maybe something shorter :) Maybe odbc_no_named_ph ?
The new preparse() stuff the DBI will be getting soon will impact
this kind of thing, so no point adding new DBI attributes just yet.
Tim.
> > However, if you're happy with this more limited solution then it's
> > fine by me. But I don't maintain the driver.
>
> Yes, but your opinion weighs in heavily :)
>
>
> > > This means that there is no need to have :: in the SAPDB code, whee!
> > >
> > >
> > > --- orig/DBD-ODBC-0.29/dbdimp.c Thu Nov 8 10:48:01 2001
> > > +++ DBD-ODBC-0.29/dbdimp.c Tue Dec 4 19:51:27 2001
> > > @@ -183,16 +183,15 @@
> > > * and level 2+ just to indicate that we are trying SQLConnect.
> > > */
> > > if (!SQL_ok(rc)) {
> > > + if (DBIS->debug > 3) {
> > > #ifdef DBD_ODBC_NO_SQLDRIVERCONNECT
> > > PerlIO_printf(DBILOGFP, "SQLDriverConnect unsupported.\n");
> > > #else
> > > - if (DBIS->debug > 3) {
> > > PerlIO_printf(DBILOGFP, "SQLDriverConnect failed:\n");
> > > AllODBCErrors(imp_dbh->henv, imp_dbh->hdbc, 0, 1);
> > > - }
> > > -
> > > #endif /* DriverConnect supported */
> > > -
> > > + }
> > > +
> > > if (DBIS->debug >= 2)
> > > PerlIO_printf(DBILOGFP, "SQLConnect '%s', '%s', '%s'\n",
> > > dbname, uid, pwd);
> > >
> > > @@ -488,18 +487,27 @@
> > > while(isDIGIT(*src))
> > > *p++ = *src++;
> > > *p = 0;
> > > +
> > > style = 1;
> > > }
> > > else if (isALNUM(*src)) { /* ':foo' */
> > > char *p = name;
> > > - *dest++ = '?';
> > >
> > > while(isALNUM(*src)) /* includes '_' */
> > > *p++ = *src++;
> > > *p = 0;
> > > +
> > > + if (*src == '.') { /* Ignore :new.field, for sapdb triggers
> > > */
> > > + *dest++ = ':';
> > > + strcpy(dest, name);
> > > + dest += strlen(name);
> > > + continue;
> > > + }
> > > +
> > > + *dest++ = '?';
> > > style = 2;
> > > }
> > >
> > > --
> > > Regards Flemming Frandsen aka. Dion/Swamp http://dion.swamp.dk
> >
>
> Jeff
>
