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

table limit range support #16

Merged
merged 7 commits into from
Nov 8, 2022
Merged

table limit range support #16

merged 7 commits into from
Nov 8, 2022

Conversation

code2prog
Copy link

I added support for offset in table limit
sample usage:
$dump->setTableLimits([ 'table_name' => [10,200], ]);

I think this may also be useful for other users. There have been several requests over the years to add this functionality.

@code2prog code2prog closed this Nov 2, 2022
@code2prog code2prog reopened this Nov 2, 2022
@back-2-95
Copy link
Member

@code2prog Thanks for the PR! Could you add some links to old issues/PRs to get some background?

@back-2-95 back-2-95 added the enhancement New feature or request label Nov 2, 2022
@code2prog
Copy link
Author

@back-2-95 ifsnop#175

return is_numeric($this->tableLimits[$tableName]) ? $this->tableLimits[$tableName] : false;
$v = false;
if (is_numeric($this->tableLimits[$tableName])) {
$v = is_numeric($this->tableLimits[$tableName]);
Copy link
Member

Choose a reason for hiding this comment

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

Should this return $this->tableLimits[$tableName]; instead returning true?

Copy link
Author

Choose a reason for hiding this comment

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

It is possible that you are right. I'll check it on Monday and change it.

Copy link
Author

Choose a reason for hiding this comment

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

@back-2-95 I changed it and added additional tests
0196b1b

Copy link
Member

Choose a reason for hiding this comment

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

Code looks valid, I'll run the tests

Copy link
Author

Choose a reason for hiding this comment

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

src/Mysqldump.php Outdated Show resolved Hide resolved
@back-2-95
Copy link
Member

Could you also update README.md section ## Table specific export limits by adding limit to posts like [0,100] ? And perhaps some sentence about offset.

@code2prog
Copy link
Author

I will add it tomorrow morning and improve the code style

@back-2-95
Copy link
Member

@code2prog This is now good to go, merging. 👏

@back-2-95 back-2-95 merged commit 6842f6d into druidfi:main Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants