You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sedona.apache.org by ji...@apache.org on 2026/09/29 06:57:28 UTC
(sedona) branch master updated: [GH-3421] Read shapefiles whose .shx content lengths do not match the .shp record headers (#3422)
This is an automated email from the ASF dual-hosted git repository.
jiayuasu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sedona.git
The following commit(s) were added to refs/heads/master by this push:
new 52fdb2673fd [GH-3421] Read shapefiles whose .shx content lengths do not match the .shp record headers (#3422)
52fdb2673fd is described below
commit 52fdb2673fd8d68de0f01ed9b8cebb2e647850e1
Author: Jia Yu <ji...@apache.org>
AuthorDate: Mon Sep 28 23:57:23 2026 -0700
[GH-3421] Read shapefiles whose .shx content lengths do not match the .shp record headers (#3422)
---
.../parseUtils/shp/ShpFileParser.java | 18 ----------
.../shapefileParser/shapes/ShapeFileReader.java | 14 +++-----
.../org/apache/sedona/sql/ShapefileTests.scala | 42 ++++++++++++++++++++++
3 files changed, 47 insertions(+), 27 deletions(-)
diff --git a/spark/common/src/main/java/org/apache/sedona/core/formatMapper/shapefileParser/parseUtils/shp/ShpFileParser.java b/spark/common/src/main/java/org/apache/sedona/core/formatMapper/shapefileParser/parseUtils/shp/ShpFileParser.java
index a1fb476ef1d..c996ffd18f4 100644
--- a/spark/common/src/main/java/org/apache/sedona/core/formatMapper/shapefileParser/parseUtils/shp/ShpFileParser.java
+++ b/spark/common/src/main/java/org/apache/sedona/core/formatMapper/shapefileParser/parseUtils/shp/ShpFileParser.java
@@ -80,24 +80,6 @@ public class ShpFileParser implements Serializable, ShapeFileConst {
return new ShpRecord(contentArray, typeID);
}
- /**
- * abstract information from record header and then copy primitive bytes data of record to a
- * primitive record.
- *
- * @return
- * @throws IOException
- */
- public ShpRecord parseRecordPrimitiveContent(int length) throws IOException {
- // get length of record content
- int contentLength = reader.readInt();
- long recordLength = 2 * (contentLength + 4);
- remainLength -= recordLength;
- int typeID = EndianUtils.swapInteger(reader.readInt());
- byte[] contentArray = new byte[length]; // exclude the 4 bytes we read for shape type
- reader.read(contentArray, 0, contentArray.length);
- return new ShpRecord(contentArray, typeID);
- }
-
/**
* abstract id number from record header
*
diff --git a/spark/common/src/main/java/org/apache/sedona/core/formatMapper/shapefileParser/shapes/ShapeFileReader.java b/spark/common/src/main/java/org/apache/sedona/core/formatMapper/shapefileParser/shapes/ShapeFileReader.java
index 9a9c5a616a0..30d8044019c 100644
--- a/spark/common/src/main/java/org/apache/sedona/core/formatMapper/shapefileParser/shapes/ShapeFileReader.java
+++ b/spark/common/src/main/java/org/apache/sedona/core/formatMapper/shapefileParser/shapes/ShapeFileReader.java
@@ -26,7 +26,6 @@ import org.apache.hadoop.mapreduce.InputSplit;
import org.apache.hadoop.mapreduce.RecordReader;
import org.apache.hadoop.mapreduce.TaskAttemptContext;
import org.apache.hadoop.mapreduce.lib.input.FileSplit;
-import org.apache.sedona.core.formatMapper.shapefileParser.parseUtils.shp.ShapeType;
import org.apache.sedona.core.formatMapper.shapefileParser.parseUtils.shp.ShpFileParser;
public class ShapeFileReader extends RecordReader<ShapeKey, ShpRecord> {
@@ -82,7 +81,8 @@ public class ShapeFileReader extends RecordReader<ShapeKey, ShpRecord> {
if (indexId == indexes.length) {
return false;
}
- // check offset, if current offset in inputStream not match with information in shx, move it
+ // check offset, if current offset in inputStream not match with information in shx, move it.
+ // Only move forward: moving back would reread a record while the .dbf reader moves on.
long pos = indexes[indexId] * 2L;
if (shpInputStream.getPos() < pos) {
long skipBytes = pos - shpInputStream.getPos();
@@ -90,15 +90,11 @@ public class ShapeFileReader extends RecordReader<ShapeKey, ShpRecord> {
throw new IOException("Failed to seek to the right place in .shp file");
}
}
- int currentLength = indexes[indexId + 1] * 2 - 4;
recordKey = new ShapeKey();
recordKey.setIndex(parser.parseRecordHeadID());
- if (currentLength >= 0) {
- recordContent = parser.parseRecordPrimitiveContent(currentLength);
- } else {
- // Ignore this index entry
- recordContent = new ShpRecord(new byte[0], ShapeType.NULL.getId());
- }
+ // Take the content length from the .shp record header, not the .shx. Some writers store the
+ // .shx content length as the .shp content length plus the 4-word record header.
+ recordContent = parser.parseRecordPrimitiveContent();
indexId += 2;
} else {
if (getProgress() >= 1) {
diff --git a/spark/common/src/test/scala/org/apache/sedona/sql/ShapefileTests.scala b/spark/common/src/test/scala/org/apache/sedona/sql/ShapefileTests.scala
index 1efb8a671ab..4ed077fa5b5 100644
--- a/spark/common/src/test/scala/org/apache/sedona/sql/ShapefileTests.scala
+++ b/spark/common/src/test/scala/org/apache/sedona/sql/ShapefileTests.scala
@@ -28,6 +28,7 @@ import org.locationtech.jts.io.{WKTReader, WKTWriter}
import org.scalatest.BeforeAndAfterAll
import java.io.File
+import java.nio.ByteBuffer
import java.nio.file.Files
import java.util.{ArrayList => JList}
import scala.collection.mutable
@@ -43,6 +44,29 @@ class ShapefileTests extends TestBaseScala with BeforeAndAfterAll {
override def afterAll(): Unit = FileUtils.deleteDirectory(new File(temporaryLocation))
+ /**
+ * Copy gis_osm_pois_free_1 to the temporary location with its .shx rewritten by `modify`, and
+ * check that reading the copy returns the same rows as reading the original.
+ */
+ private def assertReadsLikeOriginalWithShx(modify: ByteBuffer => Unit): Unit = {
+ FileUtils.cleanDirectory(new File(temporaryLocation))
+ val source = resourceFolder + "shapefiles/gis_osm_pois_free_1"
+ Seq("shp", "dbf", "prj", "cpg").foreach { ext =>
+ FileUtils.copyFile(
+ new File(s"$source/gis_osm_pois_free_1.$ext"),
+ new File(s"$temporaryLocation/gis_osm_pois_free_1.$ext"))
+ }
+ val shx =
+ ByteBuffer.wrap(Files.readAllBytes(new File(s"$source/gis_osm_pois_free_1.shx").toPath))
+ modify(shx)
+ Files.write(new File(s"$temporaryLocation/gis_osm_pois_free_1.shx").toPath, shx.array())
+
+ val expected = sparkSession.read.format("shapefile").load(source).collect()
+ val actual = sparkSession.read.format("shapefile").load(temporaryLocation).collect()
+ assert(actual.length == 12873)
+ assert(actual.sameElements(expected))
+ }
+
describe("Shapefile read tests") {
it("read gis_osm_pois_free_1") {
val shapefileDf = sparkSession.read
@@ -246,6 +270,24 @@ class ShapefileTests extends TestBaseScala with BeforeAndAfterAll {
}
}
+ it("read shapefile whose .shx content lengths include the record header") {
+ // Some writers store each .shx content length as the .shp content length plus the 4-word
+ // record header, which violates the shapefile spec.
+ assertReadsLikeOriginalWithShx { shx =>
+ (100 until shx.limit() by 8).foreach { offset =>
+ shx.putInt(offset + 4, shx.getInt(offset + 4) + 4)
+ }
+ }
+ }
+
+ it("read shapefile whose .shx repeats an earlier record offset") {
+ // Moving back to a record that was already read would pair its geometry with the next
+ // .dbf row.
+ assertReadsLikeOriginalWithShx { shx =>
+ shx.putInt(108, shx.getInt(100))
+ }
+ }
+
it("read contains_null_geom") {
val shapefileDf = sparkSession.read
.format("shapefile")