Skip to content

BCP defaults#703

Closed
mmcnabb-vms wants to merge 21 commits into
FreeTDS:masterfrom
mmcnabb-vms:bcp_defaults
Closed

BCP defaults#703
mmcnabb-vms wants to merge 21 commits into
FreeTDS:masterfrom
mmcnabb-vms:bcp_defaults

Conversation

@mmcnabb-vms

@mmcnabb-vms mmcnabb-vms commented Feb 17, 2026

Copy link
Copy Markdown
Contributor

Changes related to Sybase/ASE BCP and BCP Defaults

  • Some bugfixes to my previous submissions about LOCK DATAROWS
  • Support "-k" flag meaning to not apply defaults even if they exist
  • Test the skipping of columns in BCP. This is a Sybase feature - a format file can be used to do a "bcp in" with the data file only containing some columns; and remaining columns should behave as if NULL were being inserted, including applying default if default exists.

@mmcnabb-vms

mmcnabb-vms commented Mar 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Rebased to master. No merge conflicts with #699. All tests passed for me, in VMS and Linux, against both MSSQL 2022 and ASE 16.0 SP4 . With caveats as per #710

mmcnabb-vms and others added 15 commits April 22, 2026 11:57
@mmcnabb-vms
bcp: (ASE) Correctly detect Datarows Locking for temporary tables
982a89f 
Signed-off-by: Matt McNabb <matthew.mcnabb@vmssoftware.com>
@mmcnabb-vms
bcp in: (ASE) Adjust row buffer size formula to allow for datarows-lo…
8224c49 
…cked format

Signed-off-by: Matt McNabb <matthew.mcnabb@vmssoftware.com>
@mmcnabb-vms
bcp in: (ASE) Remove redundant initial allocation of row buffer
406fc3b 
Signed-off-by: Matt McNabb <matthew.mcnabb@vmssoftware.com>
@mmcnabb-vms
bcp in: Comment on use of "insert bulk" with ASE
9a0d716 
Signed-off-by: Matt McNabb <matthew.mcnabb@vmssoftware.com>
@mmcnabb-vms
tdsdump_col: refactor for efficiency, and add more display types
24afcc7 
Signed-off-by: Matt McNabb <matthew.mcnabb@vmssoftware.com>
@mmcnabb-vms
Debug logging when reading default values
f83a8df 
Signed-off-by: Matt McNabb <matthew.mcnabb@vmssoftware.com>
@mmcnabb-vms
Update d_bcp to test default values
2d3afaa 
Signed-off-by: Matt McNabb <matthew.mcnabb@vmssoftware.com>
@mmcnabb-vms
bcp in: Uninitialized columns default to NULL
8747fdb 
	Previously defaulted to a zero-length value, which
	is not even valid data for some column types.

Signed-off-by: Matt McNabb <matthew.mcnabb@vmssoftware.com>
@mmcnabb-vms
t0016: defaults test can run against both database types. Include dat…
7ebec76 
…etime

	MSSQL does allow NULL to be sent for non-nullable VARCHAR.

Signed-off-by: Matt McNabb <matthew.mcnabb@vmssoftware.com>
@mmcnabb-vms
bcp: allow multiple hints, and don't validate hints
224213f 
    The validation code didn't support multiple hints,
    and it truncated hints with arguments (e.g. ORDER).

    The server will tell us anyway if a hint is invalid
    so it doesn't seem necessary to validate at compile-time.

Signed-off-by: Matt McNabb <matthew.mcnabb@vmssoftware.com>
@mmcnabb-vms
freebcp: Implement -k (meaning -hKEEP_NULLS)
24d162a 
Signed-off-by: Matt McNabb <matthew.mcnabb@vmssoftware.com>
@mmcnabb-vms
t0016: test KEEP_NULLS hint
51b2c72 
Signed-off-by: Matt McNabb <matthew.mcnabb@vmssoftware.com>
@mmcnabb-vms
t0016: test Sybase fixed column non-nullable default
6159950 
Signed-off-by: Matt McNabb <matthew.mcnabb@vmssoftware.com>
@mmcnabb-vms
t0016: allow skipping columns
c1b8b2c 
Signed-off-by: Matt McNabb <matthew.mcnabb@vmssoftware.com>
@mmcnabb-vms
t0016: test omitting bcp columns (Sybase only)
d23d94d 
	Simulates use of a Format File with missing columns.

Signed-off-by: Matt McNabb <matthew.mcnabb@vmssoftware.com>
@mmcnabb-vms

mmcnabb-vms commented Apr 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Rebased to master. One line of code had to be changed as convert.h was moved.

Note, the first four commits are not to do with BCP defaults; they are fixes to ASE datarows-locking support, including fixing a buffer overflow. Could be merged first before considering the rest of this PR.

freddy77
freddy77 reviewed Apr 22, 2026
View reviewed changes
Comment thread src/tds/bulk.c Outdated
freddy77
freddy77 reviewed Apr 22, 2026
View reviewed changes
Comment thread src/tds/bulk.c Outdated
* in a normal (non-bcp) query, which holds unpacked row data.
*/
bindinfo->current_row = NULL;
bindinfo->row_size = 0;

@freddy77 freddy77 Apr 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just remove the initialization line above.

@mmcnabb-vms mmcnabb-vms May 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. In that case perhaps my additional comment lines 202-205 would be better suited to a commit message than a code comment then

@freddy77 freddy77 May 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to close this pull request.
I suppose you can post another one about this.

freddy77
freddy77 reviewed Apr 22, 2026
View reviewed changes
Comment thread src/tds/bulk.c Outdated
freddy77
freddy77 reviewed Apr 22, 2026
View reviewed changes
Comment thread src/tds/log.c Outdated
freddy77
freddy77 reviewed Apr 22, 2026
View reviewed changes
Comment thread src/apps/freebcp.h
freddy77
freddy77 reviewed Apr 22, 2026
View reviewed changes
Comment thread src/tds/bulk.c
freddy77
freddy77 reviewed Apr 22, 2026
View reviewed changes
Comment thread src/tds/mem.c
freddy77
freddy77 reviewed Apr 22, 2026
View reviewed changes
Comment thread src/tds/mem.c
@freddy77

freddy77 commented Apr 22, 2026

Copy link
Copy Markdown
Contributor

Merged first 2 commits (minor style changes).

@freddy77

freddy77 commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Merged most of the PR. Just 4 smaller commit left (see open conversations above).

@mmcnabb-vms

mmcnabb-vms commented Apr 28, 2026

Copy link
Copy Markdown
Contributor Author

Merged most of the PR. Just 4 smaller commit left (see open conversations above).

Cool. I'm away the next two weeks, go ahead as you see fit on the remaining commits if you like.

@mmcnabb-vms
Revert "bcp in: (ASE) Remove redundant initial allocation of row buffer"
84f2b4d 
This reverts commit 406fc3b.
(Alternative commit will follow)
@mmcnabb-vms
(ASE) Remove redundant initial allocation of TDS5 BCP row buffer
0828765 
    bindinfo->current_row is a buffer for packing a TDS5 BCP out row.
    We will allocate space later once we have computed the required size.
    Not to be confused with resinfo->current_row which holds unpacked row data.

Signed-off-by: Matt McNabb <matthew.mcnabb@vmssoftware.com>
@mmcnabb-vms
Revert "bcp in: Comment on use of "insert bulk" with ASE"
abd906d 
This reverts commit 9a0d716.

Replacement follows.
@mmcnabb-vms
Note that 'insert bulk' is not officially supported by the databases
a55b3a3 
Signed-off-by: Matt McNabb <matthew.mcnabb@vmssoftware.com>
@mmcnabb-vms
Revert "tdsdump_col: refactor for efficiency, and add more display ty…
a29402a 
…pes"

This reverts commit 24afcc7.

Update follows.
@mmcnabb-vms
tdsdump_col to use tds_convert in order to support more types
707ce78 
Signed-off-by: Matt McNabb <matthew.mcnabb@vmssoftware.com>
@mmcnabb-vms

mmcnabb-vms commented May 25, 2026

Copy link
Copy Markdown
Contributor Author

Re. the four outstanding commits:

  • 406fc3b, 9a0d716, 24afcc7 I have submitted alternative commits along the lines your comments suggested (I decided to push a Revert and a new fix, instead of squashing, so that conversation doesn't get orphaned -- appveyor doesn't like it but the new commits cherry-pick to master with no issues for me)
  • f83a8df : awaiting your thoughs on that one, it's just a question of what to call the guard that tdsdump_col is behind. It could just be #if 0 or otherwise commented out .

@freddy77

freddy77 commented May 31, 2026

Copy link
Copy Markdown
Contributor 
* [f83a8df](https://github.com/FreeTDS/freetds/commit/f83a8dfac5fa26b9d5bff07cf6768229063a859b) : awaiting your thoughs on that one, it's just a question of what to call the guard that tdsdump_col is behind. It could just be #if 0 or otherwise commented out .

At the moment I used a new TDS_DEBUG_DATA that you need to set manually.
Maybe worth adding an option in configure/cmake (kind of --enable-insecure-dump).

freddy77
freddy77 reviewed May 31, 2026
View reviewed changes
Comment thread src/tds/bulk.c
Comment on lines -202 to -208
if (!IS_TDS7_PLUS(tds->conn)) {
bindinfo->current_row = tds_new(unsigned char, bindinfo->row_size);
if (!bindinfo->current_row)
goto cleanup;
bindinfo->row_free = tds_bcp_row_free;
}

@freddy77 freddy77 May 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was causing a straight forward crash. Fixed in the commit.

@freddy77

freddy77 commented May 31, 2026

Copy link
Copy Markdown
Contributor

If there are no objections I would close this PR.

@mmcnabb-vms

mmcnabb-vms commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

All changes merged

@mmcnabb-vms mmcnabb-vms closed this Jun 1, 2026
@mmcnabb-vms mmcnabb-vms deleted the bcp_defaults branch June 7, 2026 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@freddy77 freddy77 freddy77 left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mmcnabb-vms @freddy77