Skip to Content.
Sympa Menu

freetds - Re: [freetds] FW: 0.82 freebcp: -h option seems to be broken

freetds AT lists.ibiblio.org

Subject: FreeTDS Development Group

List archive

Chronological Thread  
  • From: Frediano Ziglio <freddy77 AT gmail.com>
  • To: FreeTDS Development Group <freetds AT lists.ibiblio.org>
  • Subject: Re: [freetds] FW: 0.82 freebcp: -h option seems to be broken
  • Date: Tue, 3 Sep 2013 12:12:48 +0200

2008/12/19 Lowden, James K <james.k.lowden AT alliancebernstein.com>

> Said Constantin Vasilyev:
> > > freebcp tempdb.dbo.foo in /tmp/foo.bcp -S MSSQLSRV -c -h
> > CHECK_CONSTRAINTS -U xxx -P yyy
> > Msg 20076, Level 7
> > bcp_init must be called before any other bcp routines
> >
> > db-lib: Unable to set hint "CHECK_CONSTRAINTS"
> >
> > I looked into the code and it appears like freebcp is trying to call
> > bcp_options before bcp_init.
> >
> > While being there, I also checked bcp_options function and found that
> > list of valid
> > bcp hints is incomplete (FIRE_TRIGGERS hint is missing).
>
> Oh, crumb. Actually, the -h option in freebcp is just broken.
>
> True, bcp_options() is called before bcp_init(). But that's technically
> OK, or was, because bcp_options() just records the hint for later use.
> The hint text is appended to the INSERT BULK statement generated by the
> user calling bcp_init().
>
> The error you stumbled on is caused by parameter checking code added
> later to bcp_options. It's been there for a year at least and you're
> the first to mention it. :-/
>
> freebcp simply grabs whatever is passed to the -h option and sends it
> verbatim to bcp_options. That was my original idea: let the user format
> the hint text and let the server accept/reject it. That was bcp.c
> revision 1.45. All it did was make sure the option text started with
> something that looked like a hint. Nothing checked that the hint text
> was completely correct. It could say "ROW_PER_BATCH IS A DUMB IDEA;
> drop sysobjects" and that's what would go to the server.
>
> ISTR an objection somewhere along the line that we shouldn't pass
> user-provided text unvetted to the server, and the code was changed to
> pass the internal static validation string instead of the user-provided
> one. Consequently if you pass "ROWS_PER_BATCH = 100", only
> "ROWS_PER_BATCH" goes to the server. Which won't work.
>
> (Honestly, don't see a problem passing the text. The freebcp user isn't
> malicious, and a malformed INSERT BULK statement will likely result only
> in the server dropping the connection, hardly a disaster.)
>
> So, as of today:
>
> 1. No hints work because they're stopped by the parameter checker.
> 2. If they were passed, only non-valued options would work because the
> text is truncated before the "=" sign.
> 3. The option list is incomplete, as you said.
>
> And there's more...
>
> The hint syntax is a mini-language of its own. Consider parsing "ORDER
> (column [ASC | DESC] [,...n])" and btw the "," is also used to separate
> successive hints. That's a job for regex, but the last time I pulled in
> regex, Frediano rewrote the code to remove the dependency. I don't want
> to see a whole mini-parser for this written with strtok(3). I'd rather
> see either "no hints without HAVE_REGEX" or "raw hints without
> HAVE_REGEX".
>
>
I would prefer if FreeTDS does not compile.

And the bcp_options interface should be changed. Sybase defines one
> measly option, BCPLABELED, that we don't even support. We should change
> the signature to:
>
> bcp_options( DBPROCESS *, bcp_option_t, char* value, int);
>
> where bcp_option_t is an enumerated type representing the hints, and the
> value is a good, old-fashioned C string.
>
> My proposal:
>
> 1. Add regex to freebcp. (This might have other benefits, e.g. support
> for CSV files.)
>

I don't remember why I removed regex but probably was a portability issue.
Windows for instance does not support regex. Also the bug "Having two kinds
of REs is a botch." is not that great. I would personally use only extended
syntax or even use PCRE (http://www.pcre.org/) library instead, it's very
portable! And well.. Perl re are even much more powerful too! Using POSIX
regex where available and another library on other systems could be a pain.

I just did a quick search for "posix regex windows" and results are not
very exiting, quite old libraries or mostly links to pcre.


> 2. freebcp parses the hints and passes an enum-value pair to
> bcp_options.
> 3. Make sure bcp_init is called first.
> 4. Support multiple hints. tds_bcpinfo::hint becomes char**.
>
> That should do it. Thanks for the problem report. I'm so glad to have
> something to work on, now that version-autodetection hasn't been
> started. Feel free to jump in any time. ;-)
>
> --jkl
>
>
Frediano




Archive powered by MHonArc 2.6.24.

Top of Page