[Bug 60219] [PATCH] FormulaParser can't parse external references when sheet name is quoted

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=60219

Dominik Stadler  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--- Comment #10 from Dominik Stadler  ---
Applied the less intrusive patch via r1776796, thanks for the work on this!

-- 
You are receiving this mail because:
You are the assignee for the bug.
-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

[Bug 60219] [PATCH] FormulaParser can't parse external references when sheet name is quoted

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=60219

IgnacioHR  changed:

   What|Removed |Added

 Status|NEEDINFO|NEW

--- Comment #9 from IgnacioHR  ---
Patch 34357 should be OK to fix this bug and does not modify the signature of
Workbook interface. I propose you incorporate patch 34357 to current version
and close this bug

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

[Bug 60219] [PATCH] FormulaParser can't parse external references when sheet name is quoted

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=60219

Javen O'Neal  changed:

   What|Removed |Added

 Status|NEW |NEEDINFO

--- Comment #8 from Javen O'Neal  ---
Are these external references saved in the xls or xlsx file? If so, then you
are correct that the HSSFWorkbook and XSSFWorkbook need a method to add a link
to an external workbook (I'm assuming it would also be valid to remove a link,
in which case we would need an add, remove and get external links methods).

If only the FormulaEvaluator needs to know about external references (if this
information is not saved in the workbook or is always auto-created when an
external reference is found), then there is no need to complicate the API with
multiple ways to link workbooks for evaluation.

Relevant: https://poi.apache.org/spreadsheet/eval.html#external

FWIW, I tested attachment 34354 and it passes with the current test suite.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

[Bug 60219] [PATCH] FormulaParser can't parse external references when sheet name is quoted

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=60219

Javen O'Neal  changed:

   What|Removed |Added

   Keywords||PatchAvailable

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

[Bug 60219] [PATCH] FormulaParser can't parse external references when sheet name is quoted

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=60219

--- Comment #7 from IgnacioHR  ---
Created attachment 34357
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34357=edit
Alternative patch to previous one

This is an alternative patch to the full patch. This one does not change the
Workbook interface. Only the TestBugs.java and FormulaParser.java

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

[Bug 60219] [PATCH] FormulaParser can't parse external references when sheet name is quoted

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=60219

IgnacioHR  changed:

   What|Removed |Added

  Attachment #34342|0   |1
is obsolete||

--- Comment #6 from IgnacioHR  ---
Created attachment 34354
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34354=edit
Full patch, passes conf suite

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

[Bug 60219] [PATCH] FormulaParser can't parse external references when sheet name is quoted

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=60219

--- Comment #5 from IgnacioHR  ---
I'm sorry to be so active at this time.

o.a.p.hssf.usermodel.TestHSSFFormulaEvaluator.java#testXRefs suggest me that
the preferred option is (2) (the external workbook must be added to the
linkTable before the setFormula is called)

This means the code o.a.p.hssf.usermodel.TestBugs#bug45970 MUST always fail and
the current situation (the test pass without a fail) pass because the
resolution of the formula reference contains a bug and the external workbook is
recognised as a worksheet of the current workbook.

In order to fix code in bug45970. The changes should be like this:

---
// Link our new workbook
wb1.linkExternalWorkbook("$http://gagravarr.org/FormulaRefs2.xls;, new String[]
{"Sheet1"});

// Change 4  
row.getCell(1).setCellFormula("'[$http://gagravarr.org/FormulaRefs2.xls]Sheet1'!B2");
row.getCell(1).setCellValue(123.0);

// Link our new workbook
wb1.linkExternalWorkbook("$http://example.com/FormulaRefs.xls;, new String[]
{"Sheet1"});

// Add 5
row = s.createRow(5);
row.createCell(1, CellType.FORMULA);
  
row.getCell(1).setCellFormula("'[$http://example.com/FormulaRefs.xls]Sheet1'!B1");
row.getCell(1).setCellValue(234.0);
---

Note I'm adding the external workbook to the current workbook prior to set a
formula that uses cells in that workbook.

The current implementation of linkExternalWorkbook requires the external
workbook to be instantiated as a Workbook object. In order to support
referencing external workbooks that are now already instantiated a new method
linkExternalWorkbook(String name, String sheetNames[]) is required.

The attached new patch contains all required changes and passes complete
conformance suite.

The changes include:

added linkExternalWorkbook(String name, String sheetNames[]) to 
src/java/org/apache/poi/ss/usermodel/Workbook.java and added implementation of
derived classes. Note XSSF... does not implements external workbooks at all.

TestBugs now can uncomment two final tests (that pass OK) to check the external
references.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

[Bug 60219] [PATCH] FormulaParser can't parse external references when sheet name is quoted

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=60219

--- Comment #4 from IgnacioHR  ---
OK, I've dig into the problem of TestBugs#bug45970 and this is what I see.

on line 1852 of o.a.p.hssf.usermodel.TestBugs.java there is the following:

row.getCell(1).setCellFormula("'[$http://gagravarr.org/FormulaRefs2.xls]Sheet1'!B2");

During setCellFormula operation, the referenced external workbook is not in the
LinkTable as you can see in the following stack trace:

LinkTable.getExternalSheetIndex(String, String, String) line: 443
InternalWorkbook.getExternalSheetIndex(String, String, String) line: 2012
HSSFEvaluationWorkbook.getSheetExtIx(SheetIdentifier) line: 300
HSSFEvaluationWorkbook.get3DReferencePtg(CellReference, SheetIdentifier) line:
94
FormulaParser.createAreaRefParseNode(SheetIdentifier,
FormulaParser$SimpleRangePart, FormulaParser$SimpleRangePart) line: 957
FormulaParser.parseRangeable() line: 568
FormulaParser.parseRangeExpression() line: 311
FormulaParser.parseSimpleFactor() line: 1516
FormulaParser.percentFactor() line: 1474
FormulaParser.powerFactor() line: 1461
FormulaParser.Term() line: 1834
FormulaParser.additiveExpression() line: 1962
FormulaParser.concatExpression() line: 1946
FormulaParser.comparisonExpression() line: 1903
FormulaParser.intersectionExpression() line: 1876
FormulaParser.unionExpression() line: 1856
FormulaParser.parse() line: 2004
FormulaParser.parse(String, FormulaParsingWorkbook, FormulaType, int, int)
line: 170
FormulaParser.parse(String, FormulaParsingWorkbook, FormulaType, int) line: 190 
HSSFFormulaParser.parse(String, HSSFWorkbook, FormulaType, int) line: 108
HSSFCell.setCellFormula(String) line: 622
TestBugs.bug45970() line: 1852


LinkTable.getExternalSheetIndex(String, String, String) line: 443 returns -1
because and on line 445 of o.a.p.hssf.model.LinkTable.java a RuntimeException
is raised.

There are three options:
1) Modify LinkTable to automatically add a new external reference to the link
table rather that raise an exception (this way looks easy for POI users)
2) Modify test case 45970 to add an external workbook reference before setting
the formula
3) Other, such as define a policy for unreferenced external workbooks and
either raise an error or create the reference automatically.

Going for (2) seems obvious and the simplest way to fix this problem, but makes
live more difficult to developers
Going for (1) seems possible in a short period of time, but might break
existing test cases (I'm not 100% sure about this)
Going for (3) seems to be the right position in a long term. But requires more
work

Any preference?

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

[Bug 60219] [PATCH] FormulaParser can't parse external references when sheet name is quoted

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=60219

--- Comment #3 from IgnacioHR  ---
Hi,

Sorry, I don't understand this part of the comment:

"Unfortunately, this patch cannot be committed yet because it breaks
o.a.p.hssf.usermodel.TestBugs#bug45970."

row.getCell(1).setCellFormula("'[$http://gagravarr.org/FormulaRefs2.xls]Sheet1'!B2");

The error message generated is right cause the Excel sheet referenced in the
URL does not exists. So maybe there is a bug in the output of test case
o.a.p.hssf.usermodel.TestBugs#bug45970 because it MUST raise that exception

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

[Bug 60219] [PATCH] FormulaParser can't parse external references when sheet name is quoted

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=60219

--- Comment #2 from Javen O'Neal  ---
Committed disabled unit test in r1763937.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

[Bug 60219] [PATCH] FormulaParser can't parse external references when sheet name is quoted

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=60219

Javen O'Neal  changed:

   What|Removed |Added

  Attachment #34334|0   |1
is obsolete||

--- Comment #1 from Javen O'Neal  ---
Created attachment 34342
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34342=edit
FormulaParser.java and unit test patch for quoted external sheet names

Thanks for the patch! I added a unit test (attached) to make sure this stays
fixed in the future.

Unfortunately, this patch cannot be committed yet because it breaks
o.a.p.hssf.usermodel.TestBugs#bug45970.

row.getCell(1).setCellFormula("'[$http://gagravarr.org/FormulaRefs2.xls]Sheet1'!B2");

> No external workbook with name '$http://gagravarr.org/FormulaRefs2.xls'
> java.lang.RuntimeException: No external workbook with name 
> '$http://gagravarr.org/FormulaRefs2.xls'
> at 
> org.apache.poi.hssf.model.LinkTable.getExternalSheetIndex(LinkTable.java:445)
> at 
> org.apache.poi.hssf.model.InternalWorkbook.getExternalSheetIndex(InternalWorkbook.java:2012)
> at 
> org.apache.poi.hssf.usermodel.HSSFEvaluationWorkbook.getSheetExtIx(HSSFEvaluationWorkbook.java:300)
> at 
> org.apache.poi.hssf.usermodel.HSSFEvaluationWorkbook.get3DReferencePtg(HSSFEvaluationWorkbook.java:94)
> at 
> org.apache.poi.ss.formula.FormulaParser.createAreaRefParseNode(FormulaParser.java:957)
> at 
> org.apache.poi.ss.formula.FormulaParser.parseRangeable(FormulaParser.java:568)
> at 
> org.apache.poi.ss.formula.FormulaParser.parseRangeExpression(FormulaParser.java:311)
> at 
> org.apache.poi.ss.formula.FormulaParser.parseSimpleFactor(FormulaParser.java:1517)
> ...
> at 
> org.apache.poi.ss.formula.FormulaParser.unionExpression(FormulaParser.java:1857)
> at 
> org.apache.poi.ss.formula.FormulaParser.parse(FormulaParser.java:2005)
> at 
> org.apache.poi.ss.formula.FormulaParser.parse(FormulaParser.java:170)
> at 
> org.apache.poi.ss.formula.FormulaParser.parse(FormulaParser.java:190)
> at 
> org.apache.poi.hssf.model.HSSFFormulaParser.parse(HSSFFormulaParser.java:108)
> at 
> org.apache.poi.hssf.usermodel.HSSFCell.setCellFormula(HSSFCell.java:622)
> at org.apache.poi.hssf.usermodel.TestBugs.bug45970(TestBugs.java:1852)

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

[Bug 60219] [PATCH] FormulaParser can't parse external references when sheet name is quoted

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=60219

Javen O'Neal  changed:

   What|Removed |Added

  Attachment #34334|0   |1
   is patch||

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org