Skip to content
This repository has been archived by the owner on Dec 16, 2022. It is now read-only.

add limit and offset to rowstreamer streamQuery. #239

Closed

Conversation

makinje16
Copy link

Description

Related Issue(s)

Checklist

  • Should this PR be backported?
  • Tests were added or are not required
  • Documentation was added or is not required

Deployment Notes

Impacted Areas in Vitess

Components that this PR will affect:

  • Query Serving
  • VReplication
  • Cluster Management
  • Build/CI
  • VTAdmin

lastpk := make([]sqltypes.Value, len(rs.pkColumns))
byteCount := 0
offset := 0
limit := 10000000
Copy link

Choose a reason for hiding this comment

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

would be good to make this size a command line configurable flag

byteCount := 0
offset := 0
limit := 10000000
delta := 10000001
Copy link

Choose a reason for hiding this comment

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

i think this ends up skipping a row due to the offset ending up being liimit + 1. consider the following example where we try to visit all rows in a set of 10 rows in a test table:

mysql> show create table bramos_repl_table;
+-------------------+----------------+
| Table             | Create Table   |
|-------------------+----------------|
| bramos_repl_table | CREATE TABLE `bramos_repl_table` (
  `date_latest` int(11) DEFAULT NULL,
  `foo` int(11) DEFAULT '0',
  `date_first` int(11) NOT NULL DEFAULT '0',
  `test_int` int(11) DEFAULT '0',
  `test_string` varchar(20) DEFAULT '',
  KEY `date_latest` (`date_latest`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4                |
+-------------------+----------------+
1 row in set
Time: 0.003s
mysql> select date_latest from bramos_repl_table  order by date_latest limit 10;
+---------------+
|   date_latest |
|---------------|
|    1621546586 |
|    1621546587 |
|    1621546588 |
|    1621546589 |
|    1621546590 |
|    1621546591 |
|    1621546592 |
|    1621546593 |
|    1621546594 |
|    1621546595 |
+---------------+
10 rows in set
Time: 0.004s
mysql> select date_latest from bramos_repl_table  order by date_latest limit 5 offset 0;
+---------------+
|   date_latest |
|---------------|
|    1621546586 |
|    1621546587 |
|    1621546588 |
|    1621546589 |
|    1621546590 |
+---------------+
5 rows in set
Time: 0.003s
mysql> select date_latest from bramos_repl_table  order by date_latest limit 5 offset 5;
+---------------+
|   date_latest |
|---------------|
|    1621546591 |
|    1621546592 |
|    1621546593 |
|    1621546594 |
|    1621546595 |
+---------------+
5 rows in set
Time: 0.004s
mysql> select date_latest from bramos_repl_table  order by date_latest limit 5 offset 6;
+---------------+
|   date_latest |
|---------------|
|    1621546592 |
|    1621546593 |
|    1621546594 |
|    1621546595 |
|    1621546596 |
+---------------+
5 rows in set
Time: 0.003s

default:
rs.sendQuery = fmt.Sprintf("%s LIMIT %d OFFSET %d", rs.sendQuery, limit, offset)
log.Infof("Streaming query: %v\n", rs.sendQuery)
gtid, err := conn.streamWithSnapshot(rs.ctx, rs.plan.Table.Name, rs.sendQuery)
Copy link

Choose a reason for hiding this comment

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

This needs to get refactored to avoid setting the isolation level and starting a transaction multiple times on the same connection: I think it ends up being fine since mysql will tell you you can't do this but we end up locking the table multiple times for no reason so it'd be great to untangle that.

Copy link

Choose a reason for hiding this comment

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

another note that i didn't even realize the first time around: after the first iteration of this loop, you'll be tacking on limit offset suffixes to a statement that already has them:

# iteration 1
sendQuery = "select * from table"
sendQuery += sendQuery + "limit $limit offset $offset"

# iteration two
sendQuery = "select * from table limit $limit offset $offset"
sendQuery += sendQuery + "limit $limit offset $offset"

@brirams
Copy link

brirams commented Mar 30, 2022

closing this out in favor of #240

@brirams
Copy link

brirams commented Mar 30, 2022

closing this out in favor of vitessio#10012

@brirams brirams closed this Mar 30, 2022
@brirams brirams closed this Mar 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants