[freetds] FW: 0.82 freebcp: -h option seems to be broken
freddy77 at gmail.com
Tue Sep 3 06:12:48 EDT 2013
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
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
> 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. ;-)
More information about the FreeTDS