Bug 38427 - (38408 follow-up) Additional fixes in rebuild_zebra.pl and koha-rebuild-zebra
Summary: (38408 follow-up) Additional fixes in rebuild_zebra.pl and koha-rebuild-zebra
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on: 38408
Blocks:
  Show dependency treegraph
 
Reported: 2024-11-12 14:46 UTC by Marcel de Rooy
Modified: 2024-11-12 23:32 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2024-11-12 14:46:16 UTC
See base report 38408.
We need to adjust koha-rebuild-zebra now to make the forks change more visible.
Fixing an issue with offset and length for biblios.
Comment 1 Marcel de Rooy 2024-11-12 14:56:26 UTC
TODO Remove obsoleted variable during dev process:
+my $chunk_size = 100000;

For the purists:
+    $offset ||= 0;
+    $length ||= ( $record_type eq 'biblio' ? Koha::Biblios->count : Koha::Authorities->count );

We should probably better do //= here. This will make -length 0 do nothing (which is useless, but anyway..).
Check on a negative offset too?
Comment 2 Marcel de Rooy 2024-11-12 15:00:36 UTC
TODO 

+            export_marc_records_from_sth( $record_type, $sth, "$directory", $nosanitize, $seq );
+            $pm->finish;    # exit for child only
+        }
+        $offset += $chunk_size;
+        $num_records_exported += $chunk_size;

Do not assume chunk_size to be equal to exported record count.
Here we need to pass back info from the child process to the parent. Since this complicates the base report, we should do it later. In daily life this wont hurt.
Comment 3 Marcel de Rooy 2024-11-12 15:03:13 UTC
TODO Resolve inconsistency in current code when using length/offset

sub select_all_authorities {
    $strsth.=qq{ LIMIT $length } if ($length && !$offset);
    $strsth.=qq{ LIMIT $offset,$length } if ($length && $offset);


sub select_all_biblios {
    $strsth.=qq{ LIMIT $length } if ($length && !$offset);
    $strsth.=qq{ LIMIT $offset,$length } if ($offset);
Comment 4 Marcel de Rooy 2024-11-12 15:14:49 UTC
(In reply to Marcel de Rooy from comment #3)
> TODO Resolve inconsistency in current code when using length/offset
> 
> sub select_all_authorities {
>     $strsth.=qq{ LIMIT $length } if ($length && !$offset);
>     $strsth.=qq{ LIMIT $offset,$length } if ($length && $offset);
> 
> 
> sub select_all_biblios {
>     $strsth.=qq{ LIMIT $length } if ($length && !$offset);
>     $strsth.=qq{ LIMIT $offset,$length } if ($offset);

Awful but passing an offset without a limit requires something like 2^64-1:
SELECT * FROM somewhere LIMIT 18446744073709551615 OFFSET 5

Surely we can pass the total number of records here like:
+    $length ||= ( $record_type eq 'biblio' ? Koha::Biblios->count : Koha::Authorities->count );
Comment 5 David Cook 2024-11-12 23:32:54 UTC
Seems that some of this would address some of my comments on bug 38408