Apache OpenOffice (AOO) Bugzilla – Issue 45843
GETPIVOTDATA function missing ...
Last modified: 2013-08-07 15:14:03 UTC
Just got a bug report about this - it seems a shame to have such a nice pivot table implementation, but then miss this [ presuambly not overly difficult to implement ] function often used to extract data before charting ;-) Sample sheet attached.
Created attachment 24203 [details] getpivotdata sample
one for the requirements team
fst: Im working on this. Any comments/ directions are appreciated :-)
Created attachment 24961 [details] Proposed patch for getpivotdata
Hi Niklas, please check the patch. Frank
Niklas - we're really just looking for some brief comments on how to re-factor this to make it acceptable :-) Personally - I'd like to see (at least) the body of ScGetPivotData, the curious query setup, the short lifetime ScDPObject safely encapsulated inside the DataPilot code itself instead of here. But of course - none of this may be necessary.
Using ScSheetSourceDesc's aQueryParam has some fundamental limitations: - It doesn't work with grouped fields (they are not in the source data that is filtered). - It doesn't work if the "Displayed value" setting is something different from "Normal" (percentages will always be 100%, as the temporary table contains no other entries). - And of course it doesn't work for tables not based on sheet data. I think it should be possible to find the values using the results that are already stored in the live table's ScDPOutput object (find the selected items using the MemberResult sequences, then look at that position in the DataResult sequence). I haven't tried it, and there might be other problems, but it looks like a better approach to me.
The xls import and export will break when loading biff8 files. In 97/2k getpivotdata had 2 args http://support.microsoft.com/?kbid=211949 It was not until biff8x in XP/2003 that the signature changed. MS did not appear to change the function ordinal
nn: Thanks for your comment :-). Ill get back with a better patch. Also will contact you on any issue....
Created attachment 25205 [details] proposed patch
nn: i have done the way you have asked. Not depending upon aQueryParam. But it returns only the total. But we should be looking at format spec in http://office.microsoft.com/en-us/assistance/HP052091071033.aspx like with filter params. My commented code, in the patch uses query param to do a query, but gets the member from memberresults, and data from dataresults. Also it now works independent of display type. ( i have set the reference to none). But it doesnt work on the database/external service mode. Need your suggestion to make it better.
What I meant was, in ScDPOutput::GetPivotData, look through the MemberResult sequences to find where in the output table the requested result is, and then return that one from the DataResult sequence, instead of always using the total result. Oh, and can you please use a tab width of 4 (or, even better, no tabs at all)? That would make your changes much easier to read.
Created attachment 34406 [details] new version of getpivotdata against m156
Beyond the specific implementation of GETPIVOTDATA testing the function highlighted a few problems with the pivot data import from xls. The first is that the function depends on the pivot tables existing to produce results. The xls importer does a recalc on import _before_ loading the pivots. I attempted a few elegant solutions to this but decided to fall back on a brute force ScDPObject::Output call after the pivot is created. As mentioned in #62490 that call forces the generation of in sheet controls. It has the additional side effect of regenerating the view of the data pilot in the sheet, which triggers a recalc for getpivottable users.
lcl_getPivotDataApplyFilter doesn't work in the patch because the "CONTINUE" handling is disturbed by result flags set for other fields (see the "Foo" column in the first attachment). But more fundamentally, the summing of results (accum) will always fail if a different summary function or "displayed value" is selected. Getpivotdata should be limited to values that are actually part of the DataPilot table, not try to reimplement subtotal calculation. Regarding Excel import: We don't refresh DataPilot tables after importing because that could overwrite cells (for example, the source data might have been changed without refreshing the pivot table in Excel).
re: CONTINUE handling. blah my other tests were working smoothly I'll examine this now to find the misunderstanding. > different summary function or "displayed value" is selected The XL variant does not support custom agregation routines. There seems to have been some attempt but it does not work as far as I can tell. By definition it returns the sum. As an extension we could consider supporting using the field aggregator, or allowing user specified aggregation in the target field specifier, but that would be an extension. > Getpivotdata should be limited to values that are actually part of the > DataPilot table This is an interesting point for debate. XL apparently has this restriction. A filter that has no visible subtotal will return a REF, but that seems like a pointless limitation. > Re: Refreshing after XL import. I realize it is a kludge. We're being hit with several layers of problem, that was a solution that will solve some of them at the expense of others. 1) Having GetPivotData depend on ScDPOutput means that it is pulling values from the _generated_ result. On xls import we do not appear to be generating based on the cached data and instead use the in sheet. Throw in a few other bugs and the DPOutput has a tenuous link to what is on the screen until we ::Output. The re are two potential solutions for GETPIVOTDATA. Either pull the data off the sheet, which seems impossibly kludgy (there is no layout information), or ensure that the DPOutput is generated using the cached values with a matching aggregation approach to Ms XL. The latter seems best, but will take some time. 2) GetPivotData is being calculated before the pivot is loaded and generating a #REF. We need to trigger a recalc for things that use them. I looked at two mechanisms. a) Keep track of all formulas that use GETPIVOTDATA on xls import and recalc them after loading the pivots. This became unyeildy as all of the potential callers were considered (cells, validation, names). b) mark the content of the pivot table as being dirty to force all things that reference it to recalc. This is less clean, but much simpler. It is also a useful side effect of regeneration. 3) It was the easiest way to do what RefreshAfterLoad + SetButtons does for the ods importer. Although it does lose us the autoformating. Solving these problems will take some time. For now, I'm advocating the 1 line sledge hammer of ScDPObject::Output.
Created attachment 34486 [details] another example
See the attached xls file for examples of different summary function or displayed value, and how getpivotdata in Excel handles them. The handling of Excel pivot tables after import is a separate issue, so we can postpone that, but shouldn't assume that the tables are refreshed.
Ahh, interesting. I was testing with different aggregators on the col/row fields which produced some nonsensical results when using the autogenerated getpivotdata feature.
There's been no activity for 6 months now. Jody, are you going to continue with this, or should someone else take over?
The plan is to do some serious reworking of the datapilot internals to convert the model from a string based to index based operation (something closer to the XL 'pivotcache'). Given that the approach in the latest patch takes an invalid approach I'll likely delay working on this until after the cache work is complete. If someone else is interested they are welcome to it.
I'm doing this myself now, using the MemberResult/DataResult approach.
*** Issue 75921 has been marked as a duplicate of this issue. ***
The implementation is in the CWS "getpivotdata".
Reassigning to QA for verification.
The following XLS file causes to memory leak if you use this patch. Please take a look.
Created attachment 45677 [details] Memory leak
rail - nn's solution is different; are you testing this cws ? or a custom build with this patch :-)
I tested with the attached patch. Let me try getpivotdata CWS in this case.
Created attachment 46177 [details] TestCaseSpecification
Created attachment 46178 [details] Testdocuments for Test Case Specification
verified in internal build cws_getpivotdata
closed because fix available in SRC680_m221
*** Issue 32305 has been marked as a duplicate of this issue. ***