Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mapi::getBlock is erasing query count on second call #2889

Closed
monetdb-team opened this issue Nov 30, 2020 · 0 comments
Closed

Mapi::getBlock is erasing query count on second call #2889

monetdb-team opened this issue Nov 30, 2020 · 0 comments

Comments

@monetdb-team
Copy link

@monetdb-team monetdb-team commented Nov 30, 2020

Date: 2011-09-20 11:39:58 +0200
From: Rémy Chibois <>
To: clients devs <>
Version: 11.5.1 (Aug2011) [obsolete]

Last updated: 2011-09-30 10:58:43 +0200

Comment 16311

Date: 2011-09-20 11:39:58 +0200
From: Rémy Chibois <>

User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_1) AppleWebKit/534.48.3 (KHTML, like Gecko) Version/5.1 Safari/534.48.3
Build Identifier:

When using DBI's fetchall_arrayref on a large result set (which spans multiple Mapi blocks), Mapi.pm's getBlock subroutine is erasing the initial count property (set on line 302 for selects) with the next block line count (line 287). That is:

--> query sent to server
--> getBlock called to fetch first reply (reply of type "1", for "initial" I presume...)
----> line 297, count property set to line count in block
----> line 302, count property set to query result line count
--> processing of rows in that block
--> getBlock called to fetch second/next reply block (reply of type "6", for "subsequent" I presume)
----> line 297, what was set in 302 is erased with the second block line count
----> as the reply is not of type "1", the total count is lost, and getReply is confused about when to stop

Reproducible: Always

Steps to Reproduce:

  1. Set-up a sample database with some data that would not fit in a single reply block size
    (my db has 60 columns et 50001 rows, mostly text and numbers, for a flat TSV size of 20MB)

  2. Use the following Perl script:

==============
!/usr/bin/env perl

use strict;
use warnings;

$|++;

use DBI();

my $dbh = DBI->connect(
'dbi:monetdb:database=testdb', 'monetdb', 'monetdb'
);

my $query = qq{
SELECT
*
FROM
sample_table
};

my $sth = $dbh->prepare($query);
$sth->execute;

Here we tell DBI to fetch at most 10000 lines (out of 50001 available)
my $r = $sth->fetchall_arrayref(undef, 10000);

Print "200 rows" in my case, should print "10000 rows"
print scalar(@{$r}) . " rows\n";

$dbh->disconnect();

Actual Results:

200 rows

Expected Results:

10000 rows

Naively commenting out line 287 seems to solve this issue, but I haven't looked everywhere else if the count property is used in other ways

Comment 16323

Date: 2011-09-23 21:13:46 +0200
From: @grobian

I think this is slightly better:

diff --git a/clients/perl/Mapi.pm b/clients/perl/Mapi.pm
--- a/clients/perl/Mapi.pm
+++ b/clients/perl/Mapi.pm
@@ -284,9 +284,9 @@ sub getBlock {
my @chars = split(//, $header);

$self->{id} = -1;
  • $self->{count} = scalar(@{$self->{lines}});
  • $self->{count} = -1;
    $self->{nrcols} = -1;
  • $self->{replysize} = $self->{count};
  • $self->{replysize} = scalar(@{$self->{lines}});
    $self->{active} = 0;
    $self->{skip} = 0; next+skip is current result row
    $self->{next} = 0; all done

That is, I don't see the need to set count to the number of returned rows, but the replysize might be necessary. I'll commit this patch, feedback appreciated.

Comment 16324

Date: 2011-09-23 21:15:27 +0200
From: @grobian

the self->{count} should of course only be removed, not set to -1

Comment 16325

Date: 2011-09-23 21:20:27 +0200
From: @grobian

Changeset 7dc06e743f7c made by Fabian Groffen fabian@cwi.nl in the MonetDB repo, refers to this bug.

For complete details, see http//devmonetdborg/hg/MonetDB?cmd=changeset;node=7dc06e743f7c

Changeset description:

Mapi.pm: don't overwrite count for continuation blocks

As suggested by R?my Chibois, don't always just overwrite the count
field, since it may wipe out the correct value for mapi continuation
blocks (with larger results).  Bug #2889

Comment 16326

Date: 2011-09-23 21:21:52 +0200
From: @grobian

Not closing, should add a test first

Comment 16330

Date: 2011-09-27 16:19:32 +0200
From: @grobian

Changeset 58ba9e3940e6 made by Fabian Groffen fabian@cwi.nl in the MonetDB repo, refers to this bug.

For complete details, see http//devmonetdborg/hg/MonetDB?cmd=changeset;node=58ba9e3940e6

Changeset description:

add test for bug #2889

Thanks R?my Chibois for his initial test snippet

Comment 16331

Date: 2011-09-27 17:27:30 +0200
From: @grobian

Changeset 688922b03344 made by Fabian Groffen fabian@cwi.nl in the MonetDB repo, refers to this bug.

For complete details, see http//devmonetdborg/hg/MonetDB?cmd=changeset;node=688922b03344

Changeset description:

test DBD-bug-2889.pl: increase output sent to client

Make sure we trigger the block problem by sending all columns to the
client instead of just one.

Comment 16348

Date: 2011-09-30 10:58:43 +0200
From: @grobian

Released in Aug2011-SP1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant