fix(Data Import): Allow parent with differing child table lengths #11223
Conversation
|
The Travis failures do not seem connected with this pull request in any way... |
gwhitney
force-pushed
the
ragged_child_tables
branch
from
910b7f3
to
0bfbd13
Compare
Prior to this commit, if a parent has multiple child tables and the child tables have different numbers of rows, then the data import will fail with an error when it detects missing mandatory fields in the (necessary) empty records in the shorter child table. This commit fixes the problem by ignoring child rows that have only INVALID_VALUES for fields relating to that child (e.g., that are altogether empty in the fields for that child). Fixes frappe#11222.
gwhitney
force-pushed
the
ragged_child_tables
branch
from
0bfbd13
to
46ac073
Compare
There was a problem hiding this comment.
@gwhitney Hey, thanks for sending this. I have made a minor change. We can return early from parse_doc if we find an empty child row. Let me know if this is fine so we can go ahead with this.
|
Thank you for looking at this in depth. Yes, it is fine to return from parse_doc if there are only invalid values. Whether it actually improves or hurts efficiency depends on how fast get_columns is or is not -- with the check as you've inserted, you have to look at every column, including the parent type columns, for each child table doctype; without that check, you only look at each column once overall, but potentially at the cost of more calls to get_columns. Since I don't have the experience to know whether get_columns is expensive, I completely defer to your judgment. As far as I am concerned, the PR as amended is fine and would be a big improvement in importing (Purchase Invoices, in particular, since they have an Items child table and a Charges and Taxes subtable, both of which are frequently used). |
|
@Mergifyio backport version-12-hotfix |
|
Command
|
Prior to this commit, if a parent has multiple child tables and
the child tables have different numbers of rows, then the
data import will fail with an error when it detects missing
mandatory fields in the (necessary) empty records in the
shorter child table.
This commit fixes the problem by ignoring child rows that have
only INVALID_VALUES for fields relating to that child (e.g.,
that are altogether empty in the fields for that child).
Closes #11222.