Skip to Content.
Sympa Menu

freetds - [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: "Lowden, James K" <james.k.lowden AT alliancebernstein.com>
  • To: "James K. Lowden" <jklowden AT schemamania.org>
  • Subject: [freetds] FW: 0.82 freebcp: -h option seems to be broken
  • Date: Fri, 19 Dec 2008 17:09:37 -0500
  • Resent-date: Fri, 19 Dec 2008 19:07:09 -0500
  • Resent-from: James K.Lowden <jklowden AT freetds.org>
  • Resent-to: TDS Development Group <freetds AT lists.ibiblio.org>

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".

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.)
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









Archive powered by MHonArc 2.6.24.

Top of Page